Skip to content
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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

krassowski
Copy link
Member

Fixes #249

@krassowski krassowski added the enhancement New feature or request label Oct 14, 2024
Cover the case where Requires-Dist is empty
@krassowski
Copy link
Member Author

Tested with:

python ~/maintainer-tools/.github/actions/base-setup/create_constraints_file.py constraints.txt $(pwd)

on jupyterlab/jupyter-ai#1029

which produced:

click==8.0
importlib-metadata==5.2.0
jsonpath-ng==1.5.3
langchain-community==0.1.0
langchain==0.1.0
typing-extensions==4.5.0
pre-commit==3.3.3
aiosqlite==0.18
deepmerge==2.0
jupyter-server==1.6
jupyterlab==4.0
traitlets==5.0
syrupy==4.0.8

And in jupyter_collaboration it produced:

jupyterlab==4.0.0
jsonschema==4.18.0
jupyter-events==0.10.0
jupyter-server-fileid==0.7.0
jupyter-server==2.4.0
jupyter-ydoc==2.0.0
pycrdt-websocket==0.14.2
importlib-metadata==4.8.3
pytest==7.0

@krassowski
Copy link
Member Author

Ready for review!

@krassowski
Copy link
Member Author

krassowski commented Oct 15, 2024

One limitation characteristic here is that it also extracts the optional dependencies. I think this was actually requested in #119

An alternative to using build.util.project_wheel_metadata which allows to exclude dependencies pinned in extras is using https://github.com/pypa/pyproject-metadata as discussed in https://discuss.python.org/t/programmatically-getting-non-optional-requirements-of-current-directory/26963/4

@krassowski
Copy link
Member Author

The issue with https://github.com/pypa/pyproject-metadata is that it only works for pyproject.toml and the current approach of project_wheel_metadata is guaranteed to not break anything because it uses the same approach as before this PR (of extracting metadata from the built wheel).

Copy link
Member

@dlqqq dlqqq left a 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]
Copy link
Member

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?

Comment on lines +8 to +14
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

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constraints file does not get created for monorepos
2 participants