madpah / requirements-parser

A Pip requirements file parser.
https://requirements-parser.readthedocs.io
Apache License 2.0
125 stars 41 forks source link

Replace deprecated pkg_resources with packaging #105

Closed oh2fih closed 1 month ago

oh2fih commented 1 month ago

Replace pkg_resources.Requirement with packaging.requirements.Requirement.

madpah commented 1 month ago

Thanks for the contribution @oh2fih. As you can see - all tests are failing - looks like you forgot to commit an updated poetry.lock file - can you include this?

oh2fih commented 1 month ago

@madpah Thanks for accepting the pipeline. I did not yet really focus on the workflows so much as I could not get their output anyway. The poetry.lock has now been updated, and the workflow is waiting for the approval again so that I can see what else is still failing.

oh2fih commented 1 month ago

Although the output looks the same, I still got this typing error:

requirements/requirement.py:249: error: Incompatible types in assignment 
(expression has type "List[List[Union[str, Any]]]", variable has type
"List[Tuple[str, str]]")  [assignment]
                req.specs = specs
                            ^
Found 1 error in 1 file (checked 1 source file)

Should I try to fix this or just blindly # type: ignore as you have done 30 times in this file already? :thinking:

This is an example of the conversion I use and its output:

>>> from packaging.requirements import Requirement as Req
>>> import re
>>> specs = []
>>> pkg_req = Req('SomeProject >= 1.2, < 2.0')
>>> for specifier in pkg_req.specifier:
...     spec = re.split('([=<>~]+)', str(specifier), maxsplit=1)
...     spec = list(filter(None, spec))
...     specs.append(spec)
... 
>>> print(specs)
[['<', '2.0'], ['>=', '1.2']]

However I test this the type is List[List[str]] and not List[Tuple[str, str]] as mypy claims:

>>> for specifier in pkg_req.specifier:
...    spec = re.split('([=<>~]+)', str(specifier), maxsplit=1)
...    spec = list(filter(None, spec))
...    print(type(spec))
...    specs.append(spec)
... 
<class 'list'>
<class 'list'>
>>> print(type(specs))
<class 'list'>
>>> for i in specs:
...    print(type(i))
... 
<class 'list'>
<class 'list'>

I'll just ignore this...

oh2fih commented 1 month ago

Getting closer!

madpah commented 1 month ago

@oh2fih - can you also see that the DCO check is not passing https://github.com/madpah/requirements-parser/pull/105/checks?check_run_id=28655859108

oh2fih commented 1 month ago

Yes, the practice of using Signed-off-by seems a bit difficult to adopt.

It seems the old library used was a bit more flexible with the line formatting and I still need to add some pre-processing. However, I am now able to run the workflows on the fork, so maybe I'll get there.

oh2fih commented 1 month ago

@madpah It is always a bit challenging to adjust to the developing environment of a new project, but the tox was actually awesome, enabling running the tests locally without manually installing all the requirements. Now I finally got all the tests to pass, and managed to use solutions that weren't too hacky, I think. :sunglasses:

oh2fih commented 1 month ago

Had I known, I would have cleaned up the commit history a bit, as now the changelog does not contain what was actually improved, the first commit message being most meaningful compared to the minor adjustments in the end.

madpah commented 1 month ago

No worries - this PR is here forever.

Thanks for the contribution @oh2fih - 0.11.0 has been released.