Skip to content

src/sage/rings: remove block-level "needs sage.foo" tags#41689

Open
orlitzky wants to merge 6 commits intosagemath:developfrom
orlitzky:remove-rings-block-needs-sage-tags
Open

src/sage/rings: remove block-level "needs sage.foo" tags#41689
orlitzky wants to merge 6 commits intosagemath:developfrom
orlitzky:remove-rings-block-needs-sage-tags

Conversation

@orlitzky
Copy link
Contributor

These are just line noise at this point. No one is maintaining them, and there's no way to install Sage without these components.

The tags eventually need to be removed if we are to stop detecting the "features" over and over again while testing.

The needs sage.libs.giac and needs sage.rings.polynomial.pbori tags were left alone because giac and brial (was: pbori) are actually optional.

@github-actions
Copy link

github-actions bot commented Feb 23, 2026

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

@orlitzky orlitzky force-pushed the remove-rings-block-needs-sage-tags branch from 9cd7d45 to 73d8488 Compare February 23, 2026 20:55
Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

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

Please fix the warnings raised by the CIs

Here is another example with an equation of degree `2`::

sage: # needs sage.libs.flint
sage: b = R.unknown()
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning : Variable 'R' referenced here was set only in doctest marked '# needs sage.libs.flint'


sage: # needs sage.modules
sage: c = LazyCombinatorialSpecies(QQ, "X").Cycles().cycle_index_series()
sage: Lplus = L(lambda n: p([1]*n), valuation=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

other warnings since variable L was set in a doctest marked '# needs sage.modules'


sage: # needs sage.libs.pari
sage: phi = R.hom([5])
sage: L._is_valid_homomorphism_(ZZ, [3], base_map=phi)
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Variable 'L' referenced here was set only in doctest marked '# needs sage.libs.pari'

@orlitzky
Copy link
Contributor Author

Sorry, I did try to address this after you pointed it out on the other PR. Github is only showing 50 warnings (max) at a time, and each warning is triggered 10x for each CI system, so more of them keep popping up as I fix others.

These are just line noise at this point. No one is maintaining them,
and there's no way to install Sage without these components.
These aren't block tags, but the block tags I removed use variables
that are declared behind these.
To avoid warnings about variables declared behind them.
@orlitzky orlitzky force-pushed the remove-rings-block-needs-sage-tags branch from e67b0bc to 4d4675f Compare February 27, 2026 19:39
Of course, standard operations are supported::

sage: # needs sage.libs.flint
sage: b = R(42/17)
Copy link
Contributor

Choose a reason for hiding this comment

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

apparently here too
Variable 'R' referenced here was set only in doctest marked '# needs sage.libs.flint'

@orlitzky
Copy link
Contributor Author

With any luck that should be all of them, since we were < 50 to begin with. Next time I'll leave the PR as a draft until I'm sure all the warnings are gone.

These keep showing up as CI warnings.
@orlitzky
Copy link
Contributor Author

Somehow, more showed up, in files the last commit did not touch 🤷

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.

3 participants