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

Add requirements files for testing, linting, and documentation #37461

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Feb 25, 2024

We add *-requirements.txt files for locking the dependencies used in ci runs. The need for such a locking was raised on the mailing list. Here we following the (or, rather, a) standard approach to this problem as outlined by @mkoeppke in https://groups.google.com/g/sage-devel/c/MIU-xo9b7pc/m/cD7__bOUAAAJ:

In the Python (pip) world:
- Version constraints of dependencies of a Python project are declared in pyproject.toml [project] dependencies (https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#declaring-required-dependency); or, equivalently. in the older formats: setup.cfg install-requires, or setup.py install_requires
- Pinning to specific versions is done using a file "requirements.txt" (https://pip.pypa.io/en/stable/reference/requirements-file-format/)
- This file can be created and updated by using "pip freeze".

The metadata is store in the pyproject.toml file added in #37446, the locked down requirements files are stored in the folder requirements (which seems to be the usual convention). The only difference is that we use a simple wrapper around pip compile ( https://github.com/jazzband/pip-tools) that transforms the abstract version constraints into pinned version specifications in requirement.txt files. This is more convienent than pip freeze as the dependencies do not have to be installed. The script can be tested with python tools/update-lock.py src (after pip install pip-tools).

The resulting setup is completely in line with the standards set by other "huge" python projects, eg https://github.com/scipy/scipy/tree/main/requirements or https://github.com/scikit-learn/scikit-learn/tree/main/build_tools/azure.

The generated requirement files are then used in the ci runs. As a positive side-effect, github will parse these requirement files and show them correctly in the dependency graph (this closes #35890).

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe mkoeppe added disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ and removed p: blocker / 1 labels Feb 25, 2024
Copy link

github-actions bot commented Mar 10, 2024

Documentation preview for this PR (built with commit 5ea446f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 14, 2024

@roed314 @jhpalmieri I set "disputed" here because when the PR was opened, I was not able to comment because Tobias had blocked me without explanation or giving notice to the sage-abuse committee. It appears that now I can comment, so I am removing "disputed".
There has not been any review yet here, so conducting a vote would seem premature.

@mkoeppe mkoeppe removed the disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ label Mar 14, 2024
@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 14, 2024

I have not looked at this proposal in more detail. But I have to note that project will not need "docs-requirements" because we have a proper solution for building the docs in:

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 17, 2024

folder requirements (which seems to be the usual convention)

Some more examples please - the PR description only gives scipy as an example that uses this folder (it was only added a few months ago).

Overall, such changes of the top-level structure (what's in SAGE_ROOT) would probably benefit from a broader discussion.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 17, 2024

As a positive side-effect, github will parse these requirement files and show them correctly in the dependency graph (this closes #35890).

It wouldn't close PR #35890 - as it only takes care of a few packages, whereas #35890 is about the full dependency graph including non-Python packages. Best to remove this closing reference from the PR description.

@mkoeppe mkoeppe removed this from the sage-10.3 milestone Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complete our GitHub dependency graph (javascript help needed)
4 participants