Resolve a few cyclic imports in categories/structure/misc #36580

tobiasdiez · 2023-10-30T01:29:39Z

Resolves the following cyclic imports:

>>> from sage.categories.sets_cat import Sets
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/sage/src/sage/categories/sets_cat.py", line 38, in <module>
    from sage.categories.category import Category
  File "/workspaces/sage/src/sage/categories/category.py", line 108, in <module>
    from sage.misc.c3_controlled import _cmp_key, _cmp_key_named, C3_sorted_merge
  File "sage/misc/c3_controlled.pyx", line 368, in init sage.misc.c3_controlled
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/category_object.pyx", line 61, in init sage.structure.category_object
ImportError: cannot import name Category

>>> from sage.categories.sets_cat import Sets
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/sage/src/sage/categories/sets_cat.py", line 38, in <module>
    from sage.categories.category import Category
  File "/workspaces/sage/src/sage/categories/category.py", line 108, in <module>
    from sage.misc.c3_controlled import _cmp_key, _cmp_key_named, C3_sorted_merge
  File "sage/misc/c3_controlled.pyx", line 368, in init sage.misc.c3_controlled
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 1, in init sage.categories.map
  File "sage/sets/pythonclass.pyx", line 17, in init sage.sets.pythonclass
ImportError: cannot import name Sets

>>> from sage.categories.sets_cat import Sets
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/sage/src/sage/categories/sets_cat.py", line 38, in <module>
    from sage.categories.category import Category
  File "/workspaces/sage/src/sage/categories/category.py", line 108, in <module>
    from sage.misc.c3_controlled import _cmp_key, _cmp_key_named, C3_sorted_merge
  File "sage/misc/c3_controlled.pyx", line 368, in init sage.misc.c3_controlled
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 24, in init sage.categories.map
  File "/workspaces/sage/src/sage/categories/homset.py", line 68, in <module>
    from sage.categories.category import Category, JoinCategory
ImportError: cannot import name 'Category' from partially initialized module 'sage.categories.category' (most likely due to a circular import) (/workspaces/sage/src/sage/categories/category.py)

>>> from sage.categories.sets_cat import Sets
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/sage/src/sage/categories/sets_cat.py", line 38, in <module>
    from sage.categories.category import Category
  File "/workspaces/sage/src/sage/categories/category.py", line 110, in <module>
    from sage.misc.unknown import Unknown
  File "/workspaces/sage/src/sage/misc/unknown.py", line 83, in <module>
    from sage.structure.unique_representation import UniqueRepresentation
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 24, in init sage.categories.map
  File "/workspaces/sage/src/sage/categories/homset.py", line 68, in <module>
    from sage.categories.category import Category, JoinCategory
ImportError: cannot import name 'Category' from partially initialized module 'sage.categories.category' (most likely due to a circular import) (/workspaces/sage/src/sage/categories/category.py)

>>> from sage.categories.sets_cat import Sets
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/sage/src/sage/categories/sets_cat.py", line 38, in <module>
    from sage.categories.category import Category
  File "/workspaces/sage/src/sage/categories/category.py", line 112, in <module>
    from sage.structure.sage_object import SageObject
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 24, in init sage.categories.map
  File "/workspaces/sage/src/sage/categories/homset.py", line 68, in <module>
    from sage.categories.category import Category, JoinCategory
ImportError: cannot import name 'Category' from partially initialized module 'sage.categories.category' (most likely due to a circular import) (/workspaces/sage/src/sage/categories/category.py)

File "sage/misc/constant_function.pyx", line 1, in init sage.misc.constant_function
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 24, in init sage.categories.map
  File "/workspaces/sage/src/sage/categories/homset.py", line 68, in <module>
    from sage.categories.category import Category, JoinCategory
  File "/workspaces/sage/src/sage/categories/category.py", line 113, in <module>
    from sage.structure.unique_representation import UniqueRepresentation
  File "/workspaces/sage/src/sage/structure/unique_representation.py", line 559, in <module>
    from sage.misc.fast_methods import WithEqualityById
  File "sage/misc/fast_methods.pyx", line 32, in init sage.misc.fast_methods
ImportError: cannot import name ConstantFunction

File "sage/misc/constant_function.pyx", line 1, in init sage.misc.constant_function
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 24, in init sage.categories.map
  File "/workspaces/sage/src/sage/categories/homset.py", line 69, in <module>
    from . import morphism
  File "sage/categories/morphism.pyx", line 42, in init sage.categories.morphism
ImportError: cannot import name ConstantFunction

File "sage/misc/constant_function.pyx", line 1, in init sage.misc.constant_function
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
  File "sage/categories/map.pyx", line 29, in init sage.categories.map
ImportError: cannot import name ConstantFunction

File "sage/misc/constant_function.pyx", line 1, in init sage.misc.constant_function
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 119, in init sage.structure.parent
  File "/workspaces/sage/src/sage/categories/sets_cat.py", line 39, in <module>
    from sage.categories.category_singleton import Category_singleton
  File "sage/categories/category_singleton.pyx", line 12, in init sage.categories.category_singleton
ImportError: cannot import name ConstantFunction

>>> import sage.categories.morphism
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "sage/categories/morphism.pyx", line 1, in init sage.categories.morphism
  File "sage/categories/map.pyx", line 1, in init sage.categories.map
  File "/workspaces/sage/src/sage/structure/__init__.py", line 2, in <module>
    import sage.structure.element
  File "sage/structure/element.pyx", line 1, in init sage.structure.element
  File "sage/structure/parent.pyx", line 1, in init sage.structure.parent
AttributeError: partially initialized module 'sage.categories.map' has no attribute 'Map' (most likely due to a circular import)

📝 Checklist

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

tobiasdiez · 2023-10-30T04:13:47Z

@mkoeppe the build on conda fails, but passes in the "Build & Test" workflow. In fact, there I don't see any recompilation of the cython files changed in this PR (e.g. category_singleton.pyx). Could you please have a look. Thanks

mkoeppe · 2023-10-30T16:34:02Z

src/sage/categories/__init__.py

@@ -0,0 +1 @@
+import sage.structure.element # resolve a cyclic import (categories.map > structure.element > ... > categories.map)


Cannot add an init file. sage.categories is a PEP-420 namespace package.

Happy to hear your alternative solution.

I'll be happy to walk you through #35095

Thanks, maybe later, for now it would be enough if you tell me how you broke this particular cyclic import.

@mkoeppe can you please explain why adding the init file here is not okay but the one you add in #37185 is okay? In both cases the package seems to be shipped by only a single distribution and doesn't need PEP-420.

the package seems to be shipped by only a single distribution

No, that's not true for sage.categories. How did you get this impression?

Which other distribution is shipping parts of categories?

You are asking for info that you know how to obtain.

I didn't find another one, that's why I am asking...

It's very strange that you should claim you didn't find another one.

It's public knowledge that you know at least 1 way how to look for them, namely by looking for files src/sage/categories/all_*.py:

In Restructure sage.*.all for modularization, replace relative by absolute imports #36676, one of the tickets that have been blocked for months by your abusive actions, you have declared that you don't like these files Restructure sage.*.all for modularization, replace relative by absolute imports #36676 (review)

mkoeppe · 2023-12-06T07:46:14Z

As explained - can't add __init__.py files.

tobiasdiez · 2023-12-06T17:03:27Z

As explained - can't add __init__.py files.

I'm still waiting for an explanation from you on how to fix these cyclic imports without the init files (and why one cannot reintroduce init files).

mkoeppe · 2023-12-06T18:20:29Z

As explained - can't add __init__.py files.

I'm still waiting for an explanation from you on how to fix these cyclic imports without the init files

That's not the duty of the reviewer.

(and why one cannot reintroduce init files).

As both of us know, I explained it to you over in #36753 (comment): We need the namespace packages for the modularization -- which is why we removed the init files.

tobiasdiez · 2023-12-07T09:29:35Z

As explained - can't add __init__.py files.

I'm still waiting for an explanation from you on how to fix these cyclic imports without the init files

That's not the duty of the reviewer.

Well you claimed to have a fix....

(and why one cannot reintroduce init files).

As both of us know, I explained it to you over in #36753 (comment):

You didn't explain anything here or there.

We need the namespace packages for the modularization.

Not true, all tests are passing perfectly fine here.

-- which is why we removed the init files.

The records show that they were removed because "There is no technical need for any of the empty init.py files. You can remove them." (#35100 (comment)). Well, here is a technical need to reintroduce one of them.

mkoeppe · 2023-12-07T18:52:28Z

The records show that they were removed because "There is no technical need for any of the empty init.py files. You can remove them." (#35100 (comment)). Well, here is a technical need to reintroduce one of them.

You are not reintroducing an empty __init__.py file.

tobiasdiez · 2023-12-08T15:45:00Z

The records show that they were removed because "There is no technical need for any of the empty init.py files. You can remove them." (#35100 (comment)). Well, here is a technical need to reintroduce one of them.

You are not reintroducing an empty __init__.py file.

Here not, but in #36753 I do.

I also don't understand why it's important (for the modularization) whether the init file is empty or not.

mkoeppe · 2023-12-08T16:38:51Z

I also don't understand why it's important (for the modularization) whether the init file is empty or not.

PEP-420 namespace packages do not have __init__.py files.

You may want to read the documentation. https://doc.sagemath.org/html/en/developer/packaging_sage_library.html

tobiasdiez · 2023-12-08T16:51:53Z

I also don't understand why it's important (for the modularization) whether the init file is empty or not.

PEP-420 namespace packages do not have __init__.py files.

I know that. But there is no universal law that tells us that we need to use namespace packages, right?

You were emphasizing that empty init files are special. In which regard are they special?

mkoeppe · 2023-12-08T16:56:00Z

You were emphasizing that empty init files are special.

I didn't.

mkoeppe · 2023-12-12T16:04:21Z

Conflicts with the modularization work.

github-actions · 2024-02-25T19:23:16Z

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

mkoeppe · 2024-03-15T06:42:10Z

@kcrisman Responding to your suggestion in https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ/m/ciVrZ7x0AQAJ:

I confirm my vote of -1 on this PR.

This PR does not solve an important problem, and reintroducing __init__.py files conflicts with the documented design of the modularized Sage library, in which PEP-420 implicit namespace packages have a central role.
In an escalation over the attempted re-introduction of empty __init__.py files in #36753, this PR here cements an obstruction, making sage.categories an ordinary (non-namespace) package. This already causes failures seen here in the Build & Test CI, section "Test modularized distribution":
https://github.com/sagemath/sage/actions/runs/8038066520/job/21955010416?pr=36580#step:15:145

 [sagemath_categories-10.3.rc0] Traceback (most recent call last):
  [sagemath_categories-10.3.rc0]   File "<string>", line 1, in <module>
  [sagemath_categories-10.3.rc0]   File "/sage/pkgs/sagemath-categories/.tox/sagepython-sagewheels-nopypi-norequirements/lib/python3.11/site-packages/sage/categories/all.py", line 34, in <module>
  [sagemath_categories-10.3.rc0]     from sage.categories.all__sagemath_objects import *
  [sagemath_categories-10.3.rc0]   File "/sage/pkgs/sagemath-categories/.tox/sagepython-sagewheels-nopypi-norequirements/lib/python3.11/site-packages/sage/categories/all__sagemath_objects.py", line 7, in <module>
  [sagemath_categories-10.3.rc0]     from sage.categories.sets_cat import Sets, EmptySetError
  [sagemath_categories-10.3.rc0]   File "/sage/pkgs/sagemath-categories/.tox/sagepython-sagewheels-nopypi-norequirements/lib/python3.11/site-packages/sage/categories/sets_cat.py", line 46, in <module>
  [sagemath_categories-10.3.rc0]     from sage.categories.algebra_functor import AlgebrasCategory
  [sagemath_categories-10.3.rc0]   File "/sage/pkgs/sagemath-categories/.tox/sagepython-sagewheels-nopypi-norequirements/lib/python3.11/site-packages/sage/categories/algebra_functor.py", line 459, in <module>
  [sagemath_categories-10.3.rc0]     from sage.categories.morphism import SetMorphism
  [sagemath_categories-10.3.rc0]   File "sage/categories/morphism.pyx", line 1, in init sage.categories.morphism (build/cythonized/sage/categories/morphism.c:15876)
  [sagemath_categories-10.3.rc0]     """
  [sagemath_categories-10.3.rc0]   File "sage/categories/map.pyx", line 1, in init sage.categories.map (build/cythonized/sage/categories/map.c:22223)
  [sagemath_categories-10.3.rc0]     r"""
  [sagemath_categories-10.3.rc0]   File "sage/structure/element.pyx", line 1, in init sage.structure.element (build/cythonized/sage/structure/element.c:46833)
  [sagemath_categories-10.3.rc0]     # Compile this with -Os because it works around a bug with
  [sagemath_categories-10.3.rc0]   File "sage/structure/parent.pyx", line 1, in init sage.structure.parent (build/cythonized/sage/structure/parent.c:33869)
  [sagemath_categories-10.3.rc0]     r"""
  [sagemath_categories-10.3.rc0] AttributeError: partially initialized module 'sage.categories.map' has no attribute 'Map' (most likely due to a circular import)

jhpalmieri · 2024-04-11T20:18:43Z

-1 from me

tobiasdiez · 2024-10-30T09:45:10Z

I still cannot use a simple category import from the modularized distribution, but I guess there is no hope that this goes in soon....

tobiasdiez added 2 commits October 30, 2023 01:28

Resolve a few cyclic imports in cattegories/structure/misc

3366282

Merge branch 'develop' into cycle_imports_cats

b8b0adc

tobiasdiez added the s: needs review label Oct 30, 2023

tobiasdiez requested review from dcoudert, kwankyu and mkoeppe October 30, 2023 01:30

tobiasdiez added s: needs work and removed s: needs review labels Oct 30, 2023

tobiasdiez added 2 commits October 30, 2023 11:17

Remove old workaround

851a510

fix import

c5dcd98

tobiasdiez changed the title ~~Resolve a few cyclic imports in cattegories/structure/misc~~ Resolve a few cyclic imports in categories/structure/misc Oct 30, 2023

mkoeppe reviewed Oct 30, 2023

View reviewed changes

tobiasdiez added 3 commits October 30, 2023 17:57

Remove old import to break structure.factory > structure.test_factory

8f2b9be

Merge remote-tracking branch 'origin/develop' into cycle_imports_cats

ffe72bb

Break import cycle rings.integer > rings.rational > rings.integer

4eede1a

vbraun force-pushed the develop branch from 883e05f to e349b00 Compare November 12, 2023 16:25

Merge branch 'develop' into cycle_imports_cats

69403ef

tobiasdiez added s: needs review and removed s: needs work labels Nov 16, 2023

Fix codestyle

b6310d9

mkoeppe added s: needs work and removed s: needs review labels Dec 6, 2023

mkoeppe added r: invalid s: needs review and removed s: needs work labels Dec 12, 2023

tobiasdiez removed the r: invalid label Dec 19, 2023

mkoeppe added the r: invalid label Dec 19, 2023

tobiasdiez added disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ and removed r: invalid labels Dec 20, 2023

Merge branch 'develop' into cycle_imports_cats

330aee7

tobiasdiez mentioned this pull request Dec 29, 2023

CI Linux: Replace use of pkill #36726

Merged

5 tasks

mkoeppe added the r: invalid label Jan 4, 2024

Merge branch 'develop' into cycle_imports_cats

ff5db49

tobiasdiez removed the r: invalid label Feb 11, 2024

Merge branch 'develop' into cycle_imports_cats

2ef50f7

mkoeppe requested a review from kiwifb March 15, 2024 06:28

mkoeppe requested review from NathanDunfield, orlitzky and saraedum March 15, 2024 22:42

Merge branch 'develop' into cycle_imports_cats

be73cc8

vbraun force-pushed the develop branch from eba5e19 to e5f42fa Compare June 3, 2024 22:15

tobiasdiez closed this Oct 30, 2024

github-actions bot removed the s: needs review label Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve a few cyclic imports in categories/structure/misc #36580

Resolve a few cyclic imports in categories/structure/misc #36580

tobiasdiez commented Oct 30, 2023 •

edited

Loading

tobiasdiez commented Oct 30, 2023 •

edited

Loading

mkoeppe Oct 30, 2023

tobiasdiez Oct 30, 2023

mkoeppe Oct 30, 2023

tobiasdiez Oct 31, 2023

tobiasdiez Feb 11, 2024

mkoeppe Feb 11, 2024

tobiasdiez Feb 12, 2024

mkoeppe Feb 12, 2024

tobiasdiez Feb 12, 2024

mkoeppe Feb 18, 2024 •

edited

Loading

mkoeppe commented Dec 6, 2023

tobiasdiez commented Dec 6, 2023

mkoeppe commented Dec 6, 2023

tobiasdiez commented Dec 7, 2023

mkoeppe commented Dec 7, 2023

tobiasdiez commented Dec 8, 2023

mkoeppe commented Dec 8, 2023

tobiasdiez commented Dec 8, 2023

mkoeppe commented Dec 8, 2023

mkoeppe commented Dec 12, 2023

github-actions bot commented Feb 25, 2024 •

edited

Loading

mkoeppe commented Mar 15, 2024

jhpalmieri commented Apr 11, 2024

tobiasdiez commented Oct 30, 2024

		@@ -0,0 +1 @@
		import sage.structure.element # resolve a cyclic import (categories.map > structure.element > ... > categories.map)

Resolve a few cyclic imports in categories/structure/misc #36580

Resolve a few cyclic imports in categories/structure/misc #36580

Conversation

tobiasdiez commented Oct 30, 2023 • edited Loading

📝 Checklist

⌛ Dependencies

tobiasdiez commented Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkoeppe Feb 18, 2024 • edited Loading

Choose a reason for hiding this comment

mkoeppe commented Dec 6, 2023

tobiasdiez commented Dec 6, 2023

mkoeppe commented Dec 6, 2023

tobiasdiez commented Dec 7, 2023

mkoeppe commented Dec 7, 2023

tobiasdiez commented Dec 8, 2023

mkoeppe commented Dec 8, 2023

tobiasdiez commented Dec 8, 2023

mkoeppe commented Dec 8, 2023

mkoeppe commented Dec 12, 2023

github-actions bot commented Feb 25, 2024 • edited Loading

mkoeppe commented Mar 15, 2024

jhpalmieri commented Apr 11, 2024

tobiasdiez commented Oct 30, 2024

tobiasdiez commented Oct 30, 2023 •

edited

Loading

tobiasdiez commented Oct 30, 2023 •

edited

Loading

mkoeppe Feb 18, 2024 •

edited

Loading

github-actions bot commented Feb 25, 2024 •

edited

Loading