Rename PolynomialRing_general to PolynomialRing_generic

[grhkm21]

Class names should be *_generic, and indeed is the case for at least 140 classes, but PolynomialRing is incorrectly suffixed with _general. This PR fixes this problem.

📝 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.

[grhkm21]

A note for future me and others: Most LSPs support renaming a symbol across an entire project. I use coc.nvim so just use coc-rename to all the names across Sage. (It doesn't detect comments though :( )

[mkoeppe]

In a somewhat different direction, we may want to look into merging constructor functions such as PolynomialRing and the "generic" classes via classcall.

[tscrim]

Normally I would completely agree with @mkoeppe about having the PolynomialRing constructor go through an ABC. However, in this case, I am quite apprehensive about it as this constructor is fairly complex, as well as currently the univariate and multivariate classes that are essentially separate:

sage: R.<x,y> = QQ[]
sage: R.__class__.__mro__
(<class 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular'>,
 <class 'sage.rings.polynomial.multi_polynomial_ring_base.MPolynomialRing_base'>,
 <class 'sage.rings.ring.CommutativeRing'>,
 <class 'sage.rings.ring.Ring'>,
 <class 'sage.structure.parent_gens.ParentWithGens'>,
 <class 'sage.structure.parent_base.ParentWithBase'>,
 <class 'sage.structure.parent_old.Parent'>,
 <class 'sage.structure.parent.Parent'>,
 <class 'sage.structure.category_object.CategoryObject'>,
 <class 'sage.structure.sage_object.SageObject'>,
 <class 'object'>)

(which doesn't even use the category framework properly too!).

@grhkm21 Just do a grep -r SAGE_ROOT/src/sage/* (possibly set to ignore generated .c files) :)

[grhkm21]

@grhkm21 Just do a grep -r SAGE_ROOT/src/sage/* (possibly set to ignore generated .c files) :)

Yeah that finds the appearance of the class, but the LSP method I mentioned renames the class + all references to it (including imports etc). Plus if there's hundreds of references, say I want to rename Ring to Rang or something, then grep will be very inaccurate, since e.g. PolynomialRing will include the word Ring. But LSP (Language + Syntax Protocol) has the context to deduce which Ring is the Ring I'm trying to rename :D

[grhkm21]

Re category: yes, a lot of things are not in the category framework. I plan to work on that whenever I have time during the summer. e.g. by replacing some isinstance(x, some ring base) or is_ring(x) by x in Rings() (locally, not PRing them), then fixing doctests. That worked once already (#37768) so it should be a good approach.

Re constructor/classcall thing: yes in this case I don't think it's feasible.

[mkoeppe]

In a somewhat different direction, we may want to look into merging constructor functions such as PolynomialRing and the "generic" classes via classcall.

I've sketched this in #38228 (for a different class)

[mkoeppe]

In light of the discussion in

• Deprecation for private and semi-private callables #38211

... we should probably also discuss a possible direction to defining suitable abstract base classes without underscores in names.

One data point: For the elements, we have introduced an ABC CommutativePolynomial.

Should we define an ABC CommutativePolynomialRing? (But would InfinitePolynomialRing be a subclass?)

[grhkm21]

@tscrim @kwankyu Hello, I want this PR to go through soon. IIRC both of you were fine with the current deprecation solution? Do you still think so? If both of you are okay let's just mark this as positive review and remove the deprecation warning soon. Or if you two agree we can even remove the deprecation warning now, and just rename this somewhat internal base class

[kwankyu]

According to the updated rules in https://doc-release--sagemath.netlify.app/html/en/developer/coding_in_python.html#deprecation, we should first decide if PolynomialRing_general is a public class or an internal class.

I think it is internal.

[tscrim]

It's in our public documentation and should not be treated as internal. Moreover, it is also such an important class that we can reasonably expect people to have imported it in their local code. Hence, it should be subject to our usual deprecation policy.

I also don't really see the point/benefit of doing this...

[kwankyu]

OK. Then it should be treated as a public class. The deprecation policy should be followed.

Otherwise I am positive to the PR.

[grhkm21]

So according to the deprecation developer guideline, I should remove the

# Placeholder class for deprecation
class PolynomialRing_general(PolynomialRing_generic):
...

placeholder, and just do

class NewClass:
    ...

OldClass = NewClass   # OldClass is deprecated. See Issue 12345.

, correct? I assume I can remove it after a year as well (adds to calendar)

[kwankyu]

Yes.

By the way, I think it should be

OldClass = NewClass   # OldClass is deprecated. See Issue #12345.

Note the added #. Indeed the typo in the developer guide seems my mistake in #38273...

[github-actions]

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

[mantepse]

LGTM

[mantepse]

Since we changed isinstance(x, ...) or isinstance(x, ...) to isinstance(x, (..., ...)) in the last beta (I think), I guess it makes sense to follow that spirit here.

[grhkm21]

Thank you! I have applied the changes :)

[mantepse]

excellent, thank you!