briancurtin / deprecation

A library to handle automated deprecations
Apache License 2.0
88 stars 31 forks source link

Restricting the packaging version to less than 20 #44

Open narenandu opened 4 years ago

narenandu commented 4 years ago

OS: CentOS7 Python version: Python2.7

Following would work with packaging-19.2 version when checked for typehinting using mypy

from packaging import version
version.parse(u"hello world")

But when packaging-20 is used there is an error originating from packaging saying the argument should strictly be a string (Text)

This PR is to restrict the packaging version to packaging-19, since mypy checks break automatically with packaging-20, which was released on Jan 5th 2020 (https://pypi.org/project/packaging/20.0/)

codecov-io commented 4 years ago

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #44   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          83     83           
  Branches       16     16           
=====================================
  Hits           83     83

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 442a041...0d99039. Read the comment docs.

briancurtin commented 4 years ago

Do you have a test case which shows that this would fail? Locally, the test suite passes with packaging==20.0.

I'm not sure restricting the packaging version is the right move instead of fixing whatever the problem is, so I think it'd be helpful to see how code using deprecation fails with it so that we can come up with a fix.

narenandu commented 4 years ago

@briancurtin , unless the tests are using mypy in someway to check for type hints, you won't be able to see the issue which I am trying to fix. But following is the output from mypy.

: error: Argument 1 to "parse" has incompatible type "unicode"; expected "str"

If you want to see the issue, please follow the steps (assuming a linux platform)

virtualenv test_case
. test_case/bin/activate
pip install packaging==20.0 mypy   # assuming you are using python3 as your /usr/bin/python
touch test.py

Then paste the content in test.py

from packaging import version
version.parse(u"hello world")

Run mypy on the file

mypy test.py

and should see the following error

: error: Argument 1 to "parse" has incompatible type "unicode"; expected "str"

But with packaging version restricted to under 20, there is no mypy error, which tells me that between the major versions of 19 and 20 of packaging, the typehints has been changed for the packaging.version.parse. Since it a breaking release (major version) from 19 to 20, thought it would nice to restrict the version of packaging.

I am open for any ideas as to how we can tackle this !

narenandu commented 4 years ago

FYI: https://github.com/pypa/packaging/releases/tag/20.0

Properly mark packaging has being fully typed by adding a py.typed file (:issue:226) Add type hints (:issue:191)

https://github.com/pypa/packaging/pull/200/files#diff-832b3493370c7de79eff39a50afebf2aR49 is the change which is making the argument mandatory str