Opened 2 years ago

Closed 23 months ago

#29881 closed enhancement (fixed)

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

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: refactoring Keywords:
Cc: tscrim Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 82676e6 (Commits, GitHub, GitLab) Commit: 82676e626a3c4ff82d3336378cb6f127ef780107
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

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.

Change History (10)

comment:1 Changed 2 years ago by tscrim

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.

comment:2 Changed 2 years ago by mkoeppe

  • Dependencies changed from #29873, #29880 to #29880

comment:3 Changed 2 years ago by mkoeppe

  • Branch set to u/mkoeppe/sage_categories__replace_imports_from_sage_rings_all_by_more_specific_imports

comment:4 Changed 2 years ago by git

  • Commit set to 82676e626a3c4ff82d3336378cb6f127ef780107

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

comment:5 Changed 2 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Dependencies #29880 deleted
  • Description modified (diff)

comment:6 Changed 2 years ago by mkoeppe

  • Status changed from new to needs_review

comment:7 Changed 2 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

Green patchbot => positive review.

comment:8 Changed 2 years ago by chapoton

  • Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Frédéric Chapoton
  • Status changed from needs_review to positive_review

bot is green

comment:9 Changed 2 years ago by mkoeppe

Thanks!

comment:10 Changed 23 months ago by vbraun

  • Branch changed from u/mkoeppe/sage_categories__replace_imports_from_sage_rings_all_by_more_specific_imports to 82676e626a3c4ff82d3336378cb6f127ef780107
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.