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

Update conda lock files #37436

Closed
wants to merge 8 commits into from
Closed

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Feb 23, 2024

📝 Checklist

Update (and minimize) the conda lock files.

  • 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

As noted in sagemath#37024 and in sagemath#36489, `sage-conf` is actually not needed on a few systems now. For this reason, we make the installation of it optional (as there are no optional install requires as far as I know, this means we remove it from `pyproject.toml` and `setup.cfg`). We also remove it from "install all dependencies via conda" as its not needed there.
@mkoeppe mkoeppe added the disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ label Feb 24, 2024
@tobiasdiez tobiasdiez added this to the sage-10.3 milestone Feb 27, 2024
@tobiasdiez
Copy link
Contributor Author

Tests are passing, would be nice to still merge it in this release cycle as it includes a few nice improvements (notably the docs currently don't build under conda due the missing inline tabs dependency that has been added in #37056 without updating the lock files).

Copy link

Documentation preview for this PR (built with commit c03018d; changes) is ready! 🎉

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@roed314
Copy link
Contributor

roed314 commented Mar 14, 2024

The Sage Code of Conduct Committee is setting this back to needs review for now based on it being part of @dimpase's reaction to our decision on #36181, #36561, #36676, #36951, #36964, #37351, #36999, and #36941. It can, of course, be granted positive review at a future date, and will be subject to the policy on disputed PRs as normal.

@saraedum
Copy link
Member

Unfortunately it's not clear what is "disputed" in this PR. (As one of the people who care a lot about conda, I'd love to cast my vote here but I would like to understand both sides first.)

Btw., I think that the PR description is somehow broken. The actual description of the PR is inside the checklist section. (While you're at it, you might want to check the check list or remove it so it's immediately clear that nothing obvious is missing from that list.)

@mkoeppe mkoeppe removed this from the sage-10.3 milestone Mar 20, 2024
@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 20, 2024

Unfortunately it's not clear what is "disputed" in this PR.

@saraedum This PR is marked as "disputed" because it bundles the disputed #37138. The latter sabotages the PyPI distribution strategy (clearly documented in https://pypi.org/project/sage-conf/).

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 20, 2024

Updating the conda-lock file does not require this bundling. Per PR description here "the configure script fails for some reason" -- this should be debugged, not papered over.

@saraedum
Copy link
Member

Since the dispute is about a dependency that is also explicitly listed as a dependency here, could the "disputed" be removed then? (Otherwise, this is imho confusing for people trying to cast votes, like myself.)

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

Since there is nothing controversial on this specific PR at the moment but rather its dependency needs a vote, I removed the disputed label.

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 5, 2024

outdated

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.

6 participants