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.

Sign up for GitHub

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

Jump to bottom

extract_number does not directly handle multiple decimal places #88

Open
ChanceNCounter opened this issue Mar 13, 2020 · 2 comments · May be fixed by #91
Open

extract_number does not directly handle multiple decimal places #88

ChanceNCounter opened this issue Mar 13, 2020 · 2 comments · May be fixed by #91
Labels
bug Something isn't working en relates to english language

Comments

@ChanceNCounter
Copy link
Contributor

Calling attention to this TODO.

I'm marking this as a bug because I think that's how users will perceive it, although one could argue that it's just unimplemented functionality.
@ChanceNCounter ChanceNCounter added the bug Something isn't working label Mar 13, 2020
@ChanceNCounter
Copy link
Contributor Author

Working fix, need to write tests and then I'll PR.

@ChanceNCounter
Copy link
Contributor Author

Update: I've improved the fix to ensure that extract_number() will stop pulling decimal places as soon as it encounters a tokenized number which was not adjacent to the previous token in the input string.

All that remains now is to communicate back up the chain to extract_numbers(), which will now need to pop a run of elements, rather than one at a time.

ChanceNCounter added a commit to ChanceNCounter/lingua-franca that referenced this issue Mar 16, 2020
@ChanceNCounter
fix MycroftAI#88: multiple decimal places
a374d79 
 * extract_number(), extract_numbers(), and all
   helper functions gain a keyword parameter
   `decimal_places` (for helpers, just `places`)
   which does what it sounds like, using builtin
   round().

 * avoid capturing non-adjacent numbers as decimal
   places

 * avoid capturing already-used decimal places as
   separate numbers in extract_numbers()

 * add a few tests for the above
This was referenced Mar 16, 2020
Open
Open
ChanceNCounter added a commit to ChanceNCounter/lingua-franca that referenced this issue Mar 24, 2020
@ChanceNCounter
fix MycroftAI#88: multiple decimal places
11fae0a 
 * extract_number(), extract_numbers(), and all
   helper functions gain a keyword parameter
   `decimal_places` (for helpers, just `places`)
   which does what it sounds like, using builtin
   round().

 * avoid capturing non-adjacent numbers as decimal
   places

 * avoid capturing already-used decimal places as
   separate numbers in extract_numbers()

 * add a few tests for the above
@JarbasAl JarbasAl added the en relates to english language label Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working en relates to english language
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix #88 and #93: multiple decimal places and multiple decimal numbers ChanceNCounter/lingua-franca
2 participants
@ChanceNCounter @JarbasAl