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

build/pkgs/gambit: Remove #37810

Merged
merged 3 commits into from
May 2, 2024
Merged

build/pkgs/gambit: Remove #37810

merged 3 commits into from
May 2, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Apr 16, 2024

Removing this broken experimental package, as proposed in 2020.

Also removing the module providing outdated "standalone instructions" to use Gambit.

Keeping the various references to Gambit tagged with # optional - gambit. This will be updated in the follow-up PR"

The motivation for opening this PR to remove gambit now (2024) is that it removes a remaining use of the function sdh_setup_bdist_wheel (the old gambit Python package could not be installed using pip). This a step for the modernization of the build frontend for Python packages in the Sage distribution:

📝 Checklist

  • The title is concise and informative.
  • 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 and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Apr 16, 2024

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 17, 2024

The failures of "CI Linux Incremental" are unrelated and already reported elsewhere.

@@ -236,8 +236,7 @@
`Gambit <http://gambit.sourceforge.net/>`_ [Gambit]_. At present this is
the only gambit algorithm available in sage but further development will
Copy link
Member

Choose a reason for hiding this comment

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

Is "available in sage" accurate? Are you just leaving it as is for the follow-up PR?

@jhpalmieri
Copy link
Member

This leaves many references to gambit, but those will presumably be dealt with in #37809. So the PR feels a little incomplete, but (a) it's easier to leave the many references intact for the follow-up, and (b) it's an improvement over the current situation because gambit just doesn't work for us right now. Is that an accurate assessment?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 25, 2024

Yes, that's the idea.

I could include a link to the follow-up #37809 in the documentation if you think that would be helpful

@jhpalmieri
Copy link
Member

I think it's fine without any more documentation, since it is an improvement over what we have now.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 26, 2024

Thanks!

vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Removing this broken experimental package, as proposed in 2020.
- Closes sagemath#30156

Also removing the module providing outdated "standalone instructions" to
use Gambit.

Keeping the various references to Gambit tagged with `# optional -
gambit`. This will be updated in the follow-up PR"
- sagemath#37809


The motivation for opening this PR to remove `gambit` now (2024) is that
it removes a remaining use of the function `sdh_setup_bdist_wheel` (the
old gambit Python package could not be installed using pip). This a step
for the modernization of the build frontend for Python packages in the
Sage distribution:
- sagemath#35618

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37810
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri
@vbraun vbraun merged commit 71d2a96 into sagemath:develop May 2, 2024
13 of 32 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone May 2, 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.

Remove broken experimental package gambit
3 participants