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.

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

Note that Locale.parse() does not accept None #978

Merged
merged 1 commit into from Mar 1, 2023
Merged

Conversation

akx
Copy link
Member

@akx akx commented Mar 1, 2023

Refs #977

@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #978 (3f3ae78) into master (56071c9) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #978      +/-   ##
==========================================
+ Coverage   90.91%   90.94%   +0.02%     
==========================================
  Files          25       25              
  Lines        4350     4350              
==========================================
+ Hits         3955     3956       +1     
+ Misses        395      394       -1     
Impacted Files Coverage Δ
babel/core.py 96.81% <ø> (+0.28%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

babel/core.py Outdated
@@ -263,7 +263,7 @@ def negotiate(
@classmethod
def parse(
cls,
identifier: str | Locale | None,
identifier: str | Locale,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit complicated here since all the LC_... constants could be None if there is a problem determining the default and they are potentially passed to this method, meaning for type checkers, it sees this function as accepting None, but always raising the exception here.

Effectively, the input hasn't changed, but now in the case where there's no default, a TypeError will occur instead of an AttributeError later on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great points, thanks! Updated the approach here to docs-only.

@akx akx merged commit 0aa54ca into master Mar 1, 2023
@akx akx deleted the locale-parse-none branch December 12, 2023 12:56
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.

None yet

2 participants