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

Improve typehints for singular_noun #186

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

zmievsa
Copy link
Contributor

@zmievsa zmievsa commented May 20, 2023

Please note that I am only fixing the problem that I personally encountered. If this pattern of "value or False" is more common in the library -- we'd need to fix those cases as well :)

@jaraco
Copy link
Owner

jaraco commented Jul 2, 2023

What is the problem you encountered?

@zmievsa
Copy link
Contributor Author

zmievsa commented Jul 2, 2023

@jaraco Let's say you want to get a singular version of the word but you don't know whether the word is already singular.

import inflect

p = inflect.engine()

def get_singular(word: str) -> str:
    new_word = p.singular_noun(word)
    if new_word is False:
        return word
    return new_word

At this point any adequate type checker will tell you that you can't return new_word because its type is str | Literal[True] which makes sense because we have only checked that it's False and your type hints indicate that it can return True.

Essentially I am just making the type hints here more specific to make sure that type checkers are not angry and for correctness.

@jaraco jaraco force-pushed the main branch 2 times, most recently from a099bff to 0bcc550 Compare July 3, 2023 18:36
@@ -24,6 +24,7 @@ include_package_data = true
python_requires = >=3.7
install_requires =
pydantic >= 1.9.1
typing_extensions >= 3.7.2
Copy link
Owner

Choose a reason for hiding this comment

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

Why 3.7.2? I looked up the changelog, but it directs to the git history for versions prior to 4.0. I don't want to add a specific version unless there was a specific reason for the minimum pertinent to this project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that Literal wasn't added until this version. Any version can be picked as long as it's after this one.

Copy link
Owner

Choose a reason for hiding this comment

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

Okay. I did confirm the introduction of Literal appears in the changelog between 3.6.6 and 3.7.4. There's no tag for 3.7.2, unfortunately, leaving it difficult to confirm that the feature was added in that version. Since we're not certain and because those releases are already years old and because this project is likely to depend on features later than that release, I'll just leave it unspecified (when I merge).

jaraco added a commit that referenced this pull request Jul 4, 2023
@jaraco jaraco merged commit d09c9a3 into jaraco:main Jul 4, 2023
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