search

python / importlib_metadata

Library to access metadata for Python packages
https://importlib-metadata.readthedocs.io
Apache License 2.0
123 stars 80 forks source link

Ensure that requirements follow PEP508 #426

Closed pelson closed 1 year ago

pelson commented 1 year ago

It looks to me that the requirements produced when reading egg-info requires.txt is trying to be converted into PEP508 compatible form, but that the implementation doesn't follow the spec with regards to extras. This PR updates the implementation to better reflect the PEP, and updates the tests accordingly.

One thing I am not clear on however:

I can't see a limitation on the requires.txt format, and believe that the following could be a valid file:

dep1
dep2

[extra1]
dep3

[extra2]
dep3

The result, I assume should be 3 lines of requirements (with the latter line being dep3[extra1,extra2]), however the existing function does not group by the dependency name, and therefore will result in 4 requirements lines. The logic for doing this grouping could get hairy for mutually exclusive markers though, and it is not clear whether this should be addressed or not (I don't have enough overview of the current state of PEPs in this area).

pelson commented 1 year ago

I notice the docs for the JSON API have the requirements specification the way it was previously: https://warehouse.pypa.io/api-reference/json.html (permalink to source https://github.com/pypi/warehouse/blob/61798559acf01681d47bb7794409e182aa4a4b9e/docs/api-reference/json.rst)

jaraco commented 1 year ago

I'm afraid I don't understand the problem and I believe the proposed change is incorrect. Please take some more time to study the difference between dep[ex] and dep; extra="ex". Both are present in PEP 508 and the two have different meanings (the former is "dep, its dependencies, and any dependencies of the ex extra of dep"; the latter is "dep, but only when the "ex" extra is indicated.").

It might help if you wrote up an issue explaining what you think the problem is, what led you to the supposed problem, what issues it creates downstream, and how you came to this understanding.

It may turn out that your proposed change here is correct, but until I understand the problem better, it's not acceptable. I'm going to close this pull request, but feel free to write up an issue answering some of the concerns. Thanks.