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

fix bug in isFractional_es and improve coverage #2356

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

ChanceNCounter
Copy link
Contributor

  • Fix: isFractional_es() parsed fractions incorrectly
  • Update: earlier commit msg suggested another fix:
    • Month parsing not fixed
    • Several failing tests (skipped) document problem
    • A TODO and an issue also created
  • Substantially improve parse_es.py test coverage
  • TODO or comment several found bugs
    • Many lines remain uncovered, incl possible bugs

Description

parse_es.isFractional_es() was pulling the wrong indices
from an array, due to old logic expecting array indices to
correspond to the Spanish words for fractions. The array
has been replaced with a dictionary, and the call to the
array reflects this.

How to test

See Coveralls.

Contributor license agreement signed?

CLA [ yes ]

- Fix: isFractional_es() parsed fractions incorrectly
- Update: earlier commit msg suggested another fix:
  - Month parsing not fixed
  - Several failing tests (skipped) document problem
  - TODO and an issue also created
- Substantially improve parse_es.py test coverage
- TODO or comment several found bugs
  - Many lines remain uncovered, incl possible bugs
@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 5, 2019
@forslund
Copy link
Collaborator

Sorry for the delay on reviewing this one. It looks good so I'll just merge it and further work can be made to finish it up later.

@forslund forslund merged commit 324fe98 into MycroftAI:dev Oct 14, 2019
@ChanceNCounter ChanceNCounter deleted the spanish-language-parsers branch October 14, 2019 14:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants