Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

steno_dictionary: fix StenoDictionaryCollection.casereverse_lookup #1066

Conversation

benoit-pierre
Copy link
Member

Don't stop at the first result, potentially ignoring lower priority dictionaries. This also tweak the StenoDictionary code to be slightly faster on load.

Timings:

  • main.json (4.02 MiB, 147424 entries): 0.781s -> 0.625s
  • german_palantype.json (58.29 MiB, 1466824 entries): 13.492s -> 10.511s
  • magnum.json (18.81 MiB, 616946 entries): 5.053s -> 4.177s
  • magnum.rtf (45.8 MiB, 616946 entries): 22.953s -> 22.731s (yes RTF is still slow as hell, much better with the new parser: 14.338s)

Don't stop at the first result, potentially ignoring lower priority
dictionaries. This also tweak the `StenoDictionary` code to be slightly
faster on load.
@morinted
Copy link
Member

Is it possible returning a set will break some external usage in plugins?

I guess only if they were mutating the returned value. Should be okay right?

@benoit-pierre
Copy link
Member Author

I think it should be okay, yes. Better to change the API now than after 4.0.0 in any case!

@morinted morinted merged commit d672e28 into openstenoproject:master Jun 2, 2019
@benoit-pierre benoit-pierre deleted the fix_steno_dictionary_casereverse branch June 5, 2019 17:00
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.

2 participants