Skip to content

Conversation

@Shavian-info
Copy link
Contributor

Added new spellings and brought existing spellings into line with the current Kingsley Read Lexicon.

Added new spellings and brought existing spellings into line with the current Kingsley Read Lexicon.
@keyman-server
Copy link

Thank you for your pull request. You'll see a "build failed" message until the Keyman team has reviewed the pull request and manually initiated the build process.

Every change committed to this branch will become part of this pull request. When you have finished submitting files and are ready for the Keyman team to review this pull request, please post a "Ready for review" comment.

@DavidLRowe
Copy link
Collaborator

This PR is in good shape. There are a few small changes (some of which will help make future updates easier) and one question.

The LICENSE.md file needs updating:

  • Change "2021" to "2021-2025". (This is now the only file where we require the year (or year range) in the copyright statement. Omiting the year in other copyright statements means files don't need to be updated merely to change the year.)

In the README.md file, please

  • remove version number (so that this file doesn't need to be changed whenever you update this model)
  • remove copyright statement (same reason), or at lease remove the year from the copyright statement
  • update the description, for example: "Lexical model based on Shaw ReadLex words" or whatever makes sense as a short description of this lexical model. (If you want to include more information about the source of the words, that's fine, but it should be generic enough that this file wouldn't normally require updating for a new version. For example, you wouldn't want to include the word count.)
  • save this description to include in the .kps file.

In the welcome.htm file, please

  • remove the "generated from template" line (and the "1.0" version number).
  • remove the date from the copyright line (that is, make it "© Shavian.info") or remove the copyright line entirely.
  • consider removing the word count (since the current list is now < 68,000) from the description.

In the .model.ts file ("Details" tab, comments field)

  • consider removing the "ReadLex 1.2 generated from template." line from the initial comment section so that you don't have to modify this file for a new version.

In the .kps file ("Details" tab)

  • For the "Welcome file" field, please select "welcome.htm" from the list of files.
  • Please change the Description from "generated from template" to the description you used in the README.md file.

I also note that the wordlist.tsv file has 8000+ words at the end with frequency count of zero. I've asked the developers what happens with these words. Do they get ignored? or given a count of "1"? or something else?

What was your intent with the zero count?

Thanks!

@Shavian-info
Copy link
Contributor Author

The word frequencies are taken from the British National Corpus. A frequency of zero means the word doesn't appear in the BNC. This may be because the word is newer than the corpus (a lot of online terms post-date the BNC) or for some reason didn't appear in the extensive source material.

I expected them simply to appear as the last option in any list of words. I can give them all a frequency of 1 if that works better for Keyman.

I'll fix those other issues you mention.

@DavidLRowe
Copy link
Collaborator

It turns out that the compiler includes words with zero counts as having no weighting, which ends up being below a weight of "1", which is exactly what you expected.

@Shavian-info
Copy link
Contributor Author

I should have now addressed all of the issues identified in #318 (comment)

Copy link
Collaborator

@DavidLRowe DavidLRowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work!

@DavidLRowe DavidLRowe merged commit 32a30c6 into keymanapp:master Jun 24, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants