🐑 Add seperate typing hints for humanize #150

coiax · 2020-08-26T11:17:25Z

Adds a collection of .pyi files that describe the expected type
signatures of the publicly available humanize functions.

A MANIFEST.in file has been added to include the new .pyi files and
the py.typed file to indicate to mypy that this package supports
typing.

mypy has been added to the pre-commit configuration to check the stubs.

Some of the signatures are slightly optimistic, since depending on error
conditions, different types are returned.

Adds a collection of `.pyi` files that describe the expected type signatures of the publicly available `humanize` functions. A `MANIFEST.in` file has been added to include the new `.pyi` files and the `py.typed` file to indicate to mypy that this package supports typing. mypy has been added to the pre-commit configuration to check the stubs. Some of the signatures are slightly optimistic, since depending on error conditions, different types are returned.

codecov-commenter · 2020-08-26T11:18:17Z

Codecov Report

Merging #150 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #150   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          554       554           
=========================================
  Hits           554       554

Flag	Coverage Δ
#GHA_Ubuntu	`100.00% <ø> (ø)`
#GHA_Windows	`100.00% <ø> (ø)`
#GHA_macOS	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/humanize/filesize.py	`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 cca5a74...1cdd1d9. Read the comment docs.

src/humanize/filesize.py

src/humanize/number.pyi

coiax · 2020-08-26T11:24:08Z

In addition, I originally started these stub files thinking you couldn't use inline typing for Python 3.5, but it turns out that was incorrect.

It might be better in that case to make the typing inline, rather than a stub; depends on what people want.

hugovk · 2020-08-30T15:18:45Z

I haven't used typing hints much in Python yet, so thanks for the PR!

Support for Python 3.5 (EOL in two weeks) has now been dropped from master, I understand 3.6 has better support for typing?

I think inline typing makes more sense than stubs, that way it sits right next to the actual code. What do you think?

A MANIFEST.in file has been added to include the new .pyi files

This project uses setuptools_scm which means all files in source control are automatically included in the sdist, so a MANIFEST.in isn't needed (unless we wanted to include generated files, or exclude something).

coiax · 2020-09-30T10:21:45Z

Ah, this one dropped off my radar; apologies.

I'll move the stubs into the function definitions themselves.

It is better to include typing hints directly in the functions that they're describing; which allows mypy to actually check the functions, both in their execution, and that they are actually returning the appropriate values. The major change here is that previously a number of the functions, if they encountered a conversion error, would return the value unchanged. However, that would require just declaring the return type as Any, which although correct, isn't particularly useful. So instead, any failed, unconvertable, inconsistent values will just have `str` called on them, and the output will returned. The function will always return a string, although sometimes it won't be in the format you expect. In addition, some internal variables inside the function have been rearranged and moved around, to avoid changing the type of an already defined variable.

hugovk

Thanks for this! I see it already found and you fixed some bugs it found!

Just a few small suggestions.

MANIFEST.in

hugovk · 2020-10-02T18:39:56Z

src/humanize/time.py

@@ -98,30 +105,29 @@ def naturaldelta(value, months=True, minimum_unit="seconds"):
    tmp = Unit[minimum_unit.upper()]
    if tmp not in (Unit.SECONDS, Unit.MILLISECONDS, Unit.MICROSECONDS):
        raise ValueError(f"Minimum unit '{minimum_unit}' not supported")
-    minimum_unit = tmp
+    min_u = tmp


Let's keep the original name here, it's more descriptive (or at least min_unit)

The problem is, the minimum_unit variable is defined in the function signature as a str, and mypy does not approve of reassigning variables to different types. I didn't want to change the name of the argument, because that could break other people calling the functions using keywords.

So the renaming is a bit awkward, but I'm not sure how else to do it.

Ah, I see.

Can we rename it to min_unit then?

src/humanize/time.py

hugovk · 2020-10-02T19:30:17Z

And of course the CI is failing:

E   AttributeError: module 'typing' has no attribute 'Literal'

hugovk · 2020-10-03T21:44:45Z

src/humanize/time.py

@@ -98,30 +105,29 @@ def naturaldelta(value, months=True, minimum_unit="seconds"):
    tmp = Unit[minimum_unit.upper()]
    if tmp not in (Unit.SECONDS, Unit.MILLISECONDS, Unit.MICROSECONDS):
        raise ValueError(f"Minimum unit '{minimum_unit}' not supported")
-    minimum_unit = tmp
+    min_u = tmp


Ah, I see.

Can we rename it to min_unit then?

src/humanize/time.py

coiax · 2020-10-07T13:06:03Z

@hugovk Would I be able to add typing_extensions to the development requirements? It contains backports of typing features that were added in versions later than Python 3.5 (like Literal). They wouldn't be imported or required in production, only during type checking.

Originally only included this file to include the `.pyi` files in the package, but it turns out it was redundant, and we don't have `.pyi` files anymore.

Instead of using `typing.Literal`, which was only added in Python 3.7, make the type annotations strings, preventing their execution in the interpreter, and use `typing_extensions` to provide Literal.

In order to make the typing accurate, the failure case was changed in multiple functions from "return the object unchanged", to "return the string representation of the object". As such, a number of tests all needed to be updated.

coiax · 2020-10-07T15:41:05Z

@hugovk Okay, we're passing CI with everything, except what appears to be a bit of an unrelated failure on windows pypy3.

Is it going to be okay changing the expected behavior of the functions from "when failing, return the original object", to "when failing, return the string representation of the object"?

If we didn't want to make the API change, we could revert the behavior, and declare the functions in question to return Any, which although correct, is less useful from a typing perspective.

hugovk · 2020-10-07T16:59:32Z

@hugovk Would I be able to add typing_extensions to the development requirements? It contains backports of typing features that were added in versions later than Python 3.5 (like Literal). They wouldn't be imported or required in production, only during type checking.

Sure, but Python 3.5 has been dropped, 3.6 is the minimum now. Is it still needed? Fine to add it if you need backports from 3.7+.

coiax · 2020-10-07T17:01:12Z

typing.Literal was new in Python 3.8, so we'll still need it.

hugovk · 2020-10-07T17:28:42Z

@hugovk Okay, we're passing CI with everything, except what appears to be a bit of an unrelated failure on windows pypy3.

Let's see how the next push goes 🤞

Is it going to be okay changing the expected behavior of the functions from "when failing, return the original object", to "when failing, return the string representation of the object"?

If we didn't want to make the API change, we could revert the behavior, and declare the functions in question to return Any, which although correct, is less useful from a typing perspective.

I'm a bit cautious. On the one hand, for success cases, we return a string anyway.

On the other, this could be considered a breaking change, and I wonder if people are relying on this behaviour.

Breaking changes are allowed with a major bump. But we just had 3.0 last week (Python 3.5 dropped), 2.0 in March (2.7 dropped) and 1.0 in February (first release in 5 years, other old versions dropped). So (if this is a breaking change) we could release 4.0 after some time with deprecation warnings.

But then on the third hand 🙃, we may end up returning odd looking strings with stuff like this:

    try:
        value = int(value)
    except (TypeError, ValueError):
        return value

And I think Django's humanize has similar behaviour. So I'm leaning towards retaining the API.

coiax · 2020-10-08T08:46:00Z

I mean, why not both? We could change the return type to Any for now, and then, for the next major release, change the API to always return a string, and update the typing definitions at the same time?

Even a small amount of typing hints will be better than no typing hints.

hugovk · 2020-10-08T15:24:49Z

Let's leave the API as it is for now, and use Any for the returns. Thank you!

rorybyrne · 2021-03-27T13:10:16Z

Any chance of getting this merged? I've got mypy complaining about humanize

hugovk · 2022-05-03T11:31:42Z

🚀 Development has moved to https://github.com/python-humanize/humanize 🚀

Please open new PRs at https://github.com/python-humanize/humanize/pulls

hugovk · 2022-05-03T16:10:31Z

@coiax I've redone this PR at python-humanize/humanize#15, please could you have a look? Thanks!

coiax commented Aug 26, 2020

View reviewed changes

src/humanize/filesize.py Show resolved Hide resolved

src/humanize/number.pyi Outdated Show resolved Hide resolved

hugovk added the changelog: Added For new features label Sep 30, 2020

coiax added 2 commits October 2, 2020 09:58

Merge branch 'master' into stubs

69076c5

hugovk reviewed Oct 2, 2020

View reviewed changes

hugovk requested changes Oct 3, 2020

View reviewed changes

coiax added 4 commits October 7, 2020 16:11

Merge branch 'master' into stubs

f4e46d9

💀 Remove non-required MANIFEST.in file

0239209

Originally only included this file to include the `.pyi` files in the package, but it turns out it was redundant, and we don't have `.pyi` files anymore.

🌉 Use typing_extensions for Literal typing

fb53a7e

Instead of using `typing.Literal`, which was only added in Python 3.7, make the type annotations strings, preventing their execution in the interpreter, and use `typing_extensions` to provide Literal.

🚨 Fix test failures through change in API

899bdfa

In order to make the typing accurate, the failure case was changed in multiple functions from "return the object unchanged", to "return the string representation of the object". As such, a number of tests all needed to be updated.

hugovk closed this May 3, 2022

hugovk mentioned this pull request May 3, 2022

Add type hints python-humanize/humanize#15

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐑 Add seperate typing hints for humanize #150

🐑 Add seperate typing hints for humanize #150

coiax commented Aug 26, 2020

codecov-commenter commented Aug 26, 2020 •

edited

coiax commented Aug 26, 2020

hugovk commented Aug 30, 2020

coiax commented Sep 30, 2020 •

edited

hugovk left a comment

hugovk Oct 2, 2020

coiax Oct 3, 2020

hugovk Oct 3, 2020

coiax Oct 7, 2020

hugovk commented Oct 2, 2020

hugovk Oct 3, 2020

coiax commented Oct 7, 2020 •

edited

coiax commented Oct 7, 2020 •

edited

hugovk commented Oct 7, 2020

coiax commented Oct 7, 2020

hugovk commented Oct 7, 2020

coiax commented Oct 8, 2020

hugovk commented Oct 8, 2020

rorybyrne commented Mar 27, 2021

hugovk commented May 3, 2022

hugovk commented May 3, 2022

🐑 Add seperate typing hints for humanize #150

🐑 Add seperate typing hints for humanize #150

Conversation

coiax commented Aug 26, 2020

codecov-commenter commented Aug 26, 2020 • edited

Codecov Report

coiax commented Aug 26, 2020

hugovk commented Aug 30, 2020

coiax commented Sep 30, 2020 • edited

hugovk left a comment

Choose a reason for hiding this comment

hugovk Oct 2, 2020

Choose a reason for hiding this comment

coiax Oct 3, 2020

Choose a reason for hiding this comment

hugovk Oct 3, 2020

Choose a reason for hiding this comment

coiax Oct 7, 2020

Choose a reason for hiding this comment

hugovk commented Oct 2, 2020

hugovk Oct 3, 2020

Choose a reason for hiding this comment

coiax commented Oct 7, 2020 • edited

coiax commented Oct 7, 2020 • edited

hugovk commented Oct 7, 2020

coiax commented Oct 7, 2020

hugovk commented Oct 7, 2020

coiax commented Oct 8, 2020

hugovk commented Oct 8, 2020

rorybyrne commented Mar 27, 2021

hugovk commented May 3, 2022

hugovk commented May 3, 2022

codecov-commenter commented Aug 26, 2020 •

edited

coiax commented Sep 30, 2020 •

edited

coiax commented Oct 7, 2020 •

edited

coiax commented Oct 7, 2020 •

edited