-
Notifications
You must be signed in to change notification settings - Fork 24
Add API to retrieve project metadata from an installed package. #116
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
base: master
Are you sure you want to change the base?
Conversation
| import os | ||
| import pathlib | ||
| import re | ||
| from importlib.metadata import metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth importing this under an alias, rather than having both metadata the import and metadata the variable in the same file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure - or we can rename some of the variables to avoid the collision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with calling the variable just "md", that's what we use for metadata at my work anyway. :-)
pyroma/projectdata.py
Outdated
| # metadata from PyPI, we just couldn't get the additional build data. | ||
| return {"_wheel_build_failed": True} | ||
|
|
||
| return normalize_metadata(metadata) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm right, this would be a bug that exists in master, but... isn't metadata not set here? Because the assignment earlier in the function raised an exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the call on L43 succeeds, then data metadata will be set on L51; if it fails, the return on L49 prevents L51 from being executed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right, sorry, I was thinking backwards.
|
I mean, this basically just calls |
Sure - but build-metadata is basically just calls to My argument would be that even though it's "just" a wrapper around importlib, that wrapper is (a) representative of a real world use case, and (b) acts as a protection against future "oh, we can refactor I've pushed an update that renames some variables to avoid the collisions, and corrects the test case that was failing (I swear that wasn't failing locally...). |
|
Well, the idea of using Pyroma as library is new to me, so... :-D What is your usecase, more specifically? |
I flagged the use case in the original PR description - Pillow incorporates a Pyroma test into it's test suite; however, that test has required modification to support iOS (see python-pillow/Pillow#9030 and python-pillow/Pillow#9116). I don't know the specific history of that test - my assumption would be that they want to assert that wheel metadata is correct for every wheel, not just the ones that are output by default on the platforms where CI is being executed. I guess one of the outcomes here could be an official "don't do that" declaration from Pyroma, and modify Pillow's CI to perform the pyroma check externally, rather than as part of the test suite. |
|
I added the Pillow test back in 2014(!) (python-pillow/Pillow#743) to prevent a packaging regression (python-pillow/Pillow#740).
Yes, we could do this. |
|
@regebro did you have any thoughts, or further questions? |
|
No, but I also did not have any time. :-D |
Adds a new
pyroma.projectdata.installed_metadata()API that allows for the extraction and normalisation of project metadata from an installed wheel.Existing APIs for
build_metadata,wheel_metadataet al work great when pyroma is being run on a project directory, as an external check of project metadata compliance.However, these mechanisms involve internally calling out to
buildto construct a project's wheel metadata from scratch. This can be problematic for 2 reasons:This PR adds a new
pyroma.projectdata.installed_metadata()API that takes the name of an installed package, and extracts the installed project metadata usingimportlib.metadata. In a CI release workflow, a test suite will (usually) be creating and installing a wheel of the project under test, so this new API allows for the extraction and normalization of that metadata, which can then be passed to a pyroma check.This was previously possible with Pyroma 4.3.3, using the
map_metadata_keys()method andimportlib.metadata; however, this method was removed in #112, and the underlying behavior factored intobuild_metadata(). This PR effectively reverses that refactoring, and adds the importlib-based usage as a convenience.The motivation for this PR comes from Pillow, which incorporates a Pyroma test into it's test suite; however, that test has required modification to support iOS (see python-pillow/Pillow#9030 and python-pillow/Pillow#9116).