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

sage.categories: Replace imports from sage.rings.all by more specific imports #29881

Closed
mkoeppe opened this issue Jun 17, 2020 · 14 comments
Closed

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Jun 17, 2020

sage.rings.all has a mixture of generic classes and implementation classes. Some of the generic and/or basic ones will probably have to be included in sage-objects (#29865). We prepare this by getting rid of imports from sage.rings.all throughout sage.categories.

CC: @tscrim

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 82676e6

Reviewer: Travis Scrimshaw, Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/29881

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jun 17, 2020
@tscrim
Copy link
Collaborator

tscrim commented Jun 17, 2020

comment:1

I have also run into import loops in a subtle way because of stuff importing from rings.all. Strong +1 on doing this broadly. There might be a slight bit of care needed for lazily important objects, but I don't remember if this was an actual issue or how big this is.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 17, 2020

Changed dependencies from #29873, #29880 to #29880

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 17, 2020

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 17, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9346d1bsage.categories.*crystals: Replace imports from sage.rings.all by more specific imports
82676e6sage.categories: Replace imports from sage.rings.all by more specific imports

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 17, 2020

Commit: 82676e6

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 17, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 17, 2020

Changed dependencies from #29880 to none

@tscrim
Copy link
Collaborator

tscrim commented Jun 17, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 17, 2020

comment:7

Green patchbot => positive review.

@fchapoton
Copy link
Contributor

comment:8

bot is green

@fchapoton
Copy link
Contributor

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Frédéric Chapoton

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 17, 2020

comment:9

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 2, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants