-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Support extracting minimum constraints for monorepo #250
base: main
Are you sure you want to change the base?
Support extracting minimum constraints for monorepo #250
Conversation
b11959c
to
73f01d4
Compare
Cover the case where Requires-Dist is empty
d31e213
to
e6de31a
Compare
Tested with:
which produced:
And in
|
Ready for review! |
One An alternative to using |
The issue with https://github.com/pypa/pyproject-metadata is that it only works for |
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.
@krassowski This PR looks good, thank you for maintaining our CI actions for monorepos like Jupyter AI! 🤗 I have a couple of questions & callouts below, but nothing blocking.
One limitation characteristic here is that it also extracts the optional dependencies. I think this was actually requested in #119
If I'm interpreting this correctly, this would be a fundamental change in the behavior of the base-setup
action, which is called in most CI jobs in Jupyter repositories. This seems unsafe, since this can cause CI failures in other repos that have optional dependencies.
On the other hand, triggering CI failures may be a desirable outcome, since it would require that maintainers keep the version floors of optional dependencies up-to-date, as well as those of required dependencies.
I see good reasons for both ways this can go, so I'm not opinionated on this. However, I think this is worth thinking through a bit more.
|
||
from build.util import project_wheel_metadata # type:ignore[import-not-found] |
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.
Do you know why this import is not found by mypy
when type checking?
if TYPE_CHECKING: | ||
# not importing this at runtime as it is only exposed in Python 3.10+ | ||
# (although the interface was already followed in earlier versions) | ||
from importlib.metadata import PackageMetadata # type:ignore[attr-defined] | ||
else: | ||
PackageMetadata = Any | ||
|
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.
(No action needed) Thanks for adding a comment here to explain things. I just noticed that the upstream documentation doesn't even seem to state the interface for PackageMetadata
: https://build.pypa.io/en/stable/api.html
Curiously, the unofficial importlib_metadata
(3rd-party implementation of the importlib.metadata
module) does have documentation for this object: https://importlib-metadata.readthedocs.io/en/latest/api.html#importlib_metadata._meta.PackageMetadata
Given all of the caveats involved with using importlib.metadata
(weird typing issues, dependence on Python version of current env), it may be worthwhile to consider using importlib_metadata
in this script instead. However, I'm not sure if that's possible given that this is a script and not its own package.
Fixes #249