Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Fix extractnumber_en failure on "%" (#2110) #2413

Merged
merged 2 commits into from
Dec 7, 2019
Merged

Fix extractnumber_en failure on "%" (#2110) #2413

merged 2 commits into from
Dec 7, 2019

Conversation

ChanceNCounter
Copy link
Contributor

Closes #2110

CLA [ yes ] (Whether you have signed a CLA - Contributor Licensing Agreement

(wait for Coveralls - test suite wouldn't run locally)

@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Dec 5, 2019
@JarbasAl
Copy link
Contributor

JarbasAl commented Dec 6, 2019 •
edited
Loading

i think it makes more sense for that to be handled in _tokenize

that will make it more useful down the line for other languages and other tasks, quick note here, this is lagging behind lingua franca since MycroftAI/lingua-franca#28

@JarbasAl JarbasAl mentioned this pull request Dec 6, 2019
Closed
@ChanceNCounter
Copy link
Contributor Author

Yeah. We should discuss the implications for Lingua Franca, as well as the other points we mentioned in the issue. I'll close this for now.

@ChanceNCounter
Copy link
Contributor Author

Discussed in chat: Jarbas suggests (and I agree) merging the hotfix, and implementing a proper fix in Lingua Franca, which will end up in Core soon enough.

@ChanceNCounter ChanceNCounter reopened this Dec 6, 2019
@JarbasAl
Copy link
Contributor

JarbasAl commented Dec 6, 2019

chat conversation here for reference https://chat.mycroft.ai/community/pl/yj486ji5gpyxtpgwsupb1rdcna

@forslund
Copy link
Collaborator

forslund commented Dec 6, 2019

Sounds good, in @JarbasAI and @ChanceNCounter I trust.

Could you cherry-pick 4016efa into this PR as well to get @ChristopherRogers1991 testcase?

@forslund
Copy link
Collaborator

forslund commented Dec 7, 2019

Excellent, thank you. Merging!

@forslund forslund merged commit d25560a into MycroftAI:dev Dec 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

extractnumber_en fails with XX%
5 participants
@ChanceNCounter @JarbasAl @forslund @devs-mycroft @ChristopherRogers1991