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

ordinal: produces different results for decimal-as-string compared to decimal-as-float input #178

Closed
jayaddison opened this issue Mar 10, 2023 · 2 comments

Comments

@jayaddison
Copy link
Contributor

Expected behaviour:

>>> from inflect import engine
>>> p = engine()
>>> p.ordinal(5.502)
'5.502nd'

Actual behaviour:

>>> from inflect import engine
>>> p = engine()
>>> p.ordinal(5.502)
'5th'

Possible cause:

Discovered by chance while adding test coverage related to ordinals for decimal numbers (if I'd represented the decimal by using a string, then I wouldn't have found this).

@jayaddison
Copy link
Contributor Author

Possible cause:

That's not all:

pydantic's validation appears to read the num: Union[int, Word] argument from the method signature and uses it to coerce the resulting num variable to type int.

If that's the case, then the ordinal method likely does not handle many float inputs correctly.

A possible fix could involve adjusting the method's signature, and/or adjusting the type hints:

    @validate_arguments(config=dict(arbitrary_types_allowed=True))  # noqa: C901
    def ordinal(self, num: Union[Number, Word]) -> str:  # noqa: C901

Todo: confirm that this is true, and then provide relevant feedback on the pydantic issue tracker.

@jaraco
Copy link
Owner

jaraco commented Jul 3, 2023

On further consideration, I don't believe I truly understood the implications of #179, and while adopting Pydantic 2, I'm having to revisit this issue, because Pydantic is now (correctly) failing when an int or float is passed but the type annotation indicates Word.

@jaraco jaraco reopened this Jul 3, 2023
@jaraco jaraco closed this as completed in 27b2157 Jul 3, 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 a pull request may close this issue.

2 participants