Repository navigation
Replace internal OrderedDict usage with dict - #1328
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1328 +/- ##
=======================================
Coverage 97.10% 97.11%
=======================================
Files 235 235
Lines 2904 2912 +8
=======================================
+ Hits 2820 2828 +8
Misses 84 84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR aims to modernize the codebase by replacing OrderedDict with plain dict now that supported Python versions preserve insertion order. It touches runtime parsing/loading code, calendar/localization helpers, data-generation scripts, and one ordering-sensitive test, in the context of issue #1108.
Changes:
- Replaces several internal
OrderedDictconstructions with{}/dict(...)in parser, locale, loader, and date-handling code. - Updates Jalali calendar metadata and locale-relative translation helpers to use plain dicts.
- Updates CLDR/supplementary data generation scripts and one date parsing test to remove
OrderedDict.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_date.py |
Rewrites an ordering-sensitive test fixture from OrderedDict to dict. |
dateparser/utils/__init__.py |
Changes combine_dicts() to build a plain dict. |
dateparser/parser.py |
Replaces directive order containers with plain dicts. |
dateparser/languages/locale.py |
Replaces relative-translation map construction with a plain dict. |
dateparser/languages/loader.py |
Replaces locale maps with plain dicts, including get_locale_map(). |
dateparser/date.py |
Changes DateDataParser.previous_locales storage to a plain dict. |
dateparser/calendars/jalali_parser.py |
Replaces ordered month/weekday maps with dict literals. |
dateparser_scripts/write_complete_data.py |
Removes OrderedDict from generated-data assembly and file loading. |
dateparser_scripts/utils.py |
Replaces script-side dict merge/diff helpers with plain dicts. |
dateparser_scripts/order_languages.py |
Builds the language-locale output map with a plain dict. |
dateparser_scripts/get_cldr_data.py |
Replaces ordered mappings in CLDR extraction/cleanup with plain dicts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| :return: ordered locale code to locale instance mapping | ||
| """ | ||
| return OrderedDict( | ||
| return dict( |
|
|
||
| def combine_dicts(primary_dict, supplementary_dict): | ||
| combined_dict = OrderedDict() | ||
| combined_dict = {} |
There was a problem hiding this comment.
The clean, backward-compatible way to approach this is probably to create a special OrderedDict subclass that triggers a warning whenever such methods are used, and after a deprecation period of ~1 year, to start returning a dict instead.
Another approach would be to deprecate the function itself, and implement a new copy of it that uses dict instead of OrderedDict. If this method should not have been a public method in the first place, implementing a _combine_dicts function that uses dict and deprecating the public one is probably the simplest approach.
get_locale_map(), dateparser.utils.combine_dicts(), and DateDataParser.previous_locales are part of the observable API, so swapping their type to plain dict would be a breaking change. Limit the OrderedDict->dict replacement to purely internal usages per issue #1108 scope and reviewer feedback.
Close #1108