Skip to content

Conversation

orlitzky
Copy link
Contributor

Drop the import of sage.rings.finite_rings.finite_field_constructor from this file, since it is unused. I noticed this because it leads to circular imports:

$ python
Python 3.13.7 (main, Aug 22 2025, 06:36:41) [GCC 15.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from sage.rings.function_field.constructor import FunctionField
>>> from sage.rings.integer import Integer
>>> from sage.rings.rational_field import QQ
>>> K = FunctionField(QQ, 'x')
Traceback (most recent call last):
...
ImportError: cannot import name PolynomialRing_generic

(The unneccessary import from sage.rings.integer is due to another, harder instance of this problem.)

Drop the import of sage.rings.finite_rings.finite_field_constructor
from this file, since it is unused. I noticed this because it leads
to circular imports:

  $ python
  Python 3.13.7 (main, Aug 22 2025, 06:36:41) [GCC 15.2.0] on linux
  Type "help", "copyright", "credits" or "license" for more information.
  >>> from sage.rings.function_field.constructor import FunctionField
  >>> from sage.rings.integer import Integer
  >>> from sage.rings.rational_field import QQ
  >>> K = FunctionField(QQ, 'x')
  Traceback (most recent call last):
  ...
  ImportError: cannot import name PolynomialRing_generic

(The unneccessary import from sage.rings.integer is due to another,
harder instance of this problem.)
Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

LGTM. Please set it to positive review as soon as CI is green as well.

Btw, do you have any suggestions on how to approach the bigger issue of resolving all those circular imports (without using sage.all as it's done currently)?

@orlitzky
Copy link
Contributor Author

It will be a painstaking process. I'm sure many instances are careless choices that had no impact at the time because everyone was using sage.all. Those can be made local at first, and perhaps reorganized later. The real problem will be with the abstract foundations (categories etc.) that all reference one another meaningfully.

I don't know why it became fatal today, but these errors are new for me I think. I made a post to sage-devel a moment ago.

Copy link

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

@orlitzky
Copy link
Contributor Author

The macos gods look upon us with favor today. The CI is all the way green. Tagging w/ positive review, thanks.

@vbraun vbraun merged commit 2d7ac64 into sagemath:develop Sep 27, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants