Skip to content

Allow to use_given_order for languages too#997

Merged
Gallaecio merged 5 commits intoscrapinghub:masterfrom
whalebot-helmsman:given_order_for_languages
Oct 27, 2021
Merged

Allow to use_given_order for languages too#997
Gallaecio merged 5 commits intoscrapinghub:masterfrom
whalebot-helmsman:given_order_for_languages

Conversation

@whalebot-helmsman
Copy link
Copy Markdown
Contributor

@whalebot-helmsman whalebot-helmsman commented Oct 7, 2021

No description provided.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 7, 2021

Codecov Report

Merging #997 (585a8db) into master (41f9478) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #997   +/-   ##
=======================================
  Coverage   98.29%   98.29%           
=======================================
  Files         234      234           
  Lines        2694     2694           
=======================================
  Hits         2648     2648           
  Misses         46       46           
Impacted Files Coverage Δ
dateparser/date.py 99.24% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41f9478...585a8db. Read the comment docs.

@whalebot-helmsman whalebot-helmsman force-pushed the given_order_for_languages branch from 507dc6d to fab5e91 Compare October 7, 2021 09:46
Comment thread dateparser/date.py Outdated
@Gallaecio
Copy link
Copy Markdown
Contributor

Is that all that needs changing? (i.e. was the feature there but this exception was getting in the way?)

Could you add a test?

@whalebot-helmsman
Copy link
Copy Markdown
Contributor Author

Is that all that needs changing? (i.e. was the feature there but this exception was getting in the way?)

Yes, e.g. for French format is dd/mm and for English it is mm/dd . English is used as a fallback, so we always add it as last one. You don't have a list of locales, just these two languages. You try to use class and get mm/dd .

To solve it I wrote this class

class OrderedDateDataParser(DateDataParser):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.use_given_order = True

And everything starts working

Could you add a test?

There is a test of loading languages with use_given_order

param(given_languages=['es', 'fr', 'en'],
given_locales=None,
given_region=None,
loaded_languages=['en', 'es', 'fr'],
loaded_locales=['en', 'es', 'fr'],
expected_locales=['es', 'fr', 'en']),

@Gallaecio
Copy link
Copy Markdown
Contributor

There is a test of loading languages with use_given_order

But it was not failing before. We need a test that would have caught the issue before your change, i.e. a test that fails if we revert your change, and passes after we apply it. And I don’t mean https://github.com/scrapinghub/dateparser/pull/997/files#diff-fd61b87100377a706498644e5ff42b8baecf18918169f6d4082af10d5a053477R658.

@Gallaecio Gallaecio merged commit 5ec1a6e into scrapinghub:master Oct 27, 2021
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