Opened 2 years ago
Last modified 4 weeks ago
#30713 needs_work enhancement
Narrow down import of SageObject
Reported by: | Tobias Diez | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.8 |
Component: | refactoring | Keywords: | sd111 |
Cc: | Matthias Köppe, Travis Scrimshaw | Merged in: | |
Authors: | Tobias Diez | Reviewers: | Travis Scrimshaw, Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | public/refactoring/narrowSageObject (Commits, GitHub, GitLab) | Commit: | ff9ed515c45cd8a66a6b2cf9f96b8604d6ae24fe |
Dependencies: | Stopgaps: |
Description
Small change of narrowing down the import of SageObject
instead of importing the whole module.
Change History (16)
comment:1 Changed 2 years ago by
Commit: | → ff9ed515c45cd8a66a6b2cf9f96b8604d6ae24fe |
---|
comment:2 Changed 2 years ago by
Status: | new → needs_review |
---|
comment:3 follow-up: 4 Changed 2 years ago by
Cc: | Travis Scrimshaw added |
---|
I also prefer the style that uses from ... import ...
, but this is a cosmetic change. It still loads the whole module.
comment:4 Changed 2 years ago by
Reviewers: | → Travis Scrimshaw |
---|
Replying to mkoeppe:
I also prefer the style that uses
from ... import ...
,
Same.
but this is a cosmetic change. It still loads the whole module.
It is not entirely cosmetic with how the imports work IIRC. But it does make is clear what is desired from that module, so I think it is a net improvement.
comment:5 Changed 2 years ago by
Reviewers: | Travis Scrimshaw → Travis Scrimshaw, Matthias Koeppe |
---|---|
Status: | needs_review → positive_review |
comment:7 Changed 2 years ago by
Milestone: | sage-9.2 → sage-9.3 |
---|
comment:8 Changed 2 years ago by
Status: | positive_review → needs_work |
---|
docbuild fails:
[dochtml] [function_] building [inventory]: targets for 11 source files that are out of date [dochtml] [function_] updating environment: [new config] 11 added, 0 changed, 0 removed [dochtml] [combinat ] /home/release/Sage/local/lib/python3.8/site-packages/sage/combinat/crystals/mv_polytopes.py:docstring of sage.combinat.crystals.mv_polytopes.MVPolytope.plot:22: WARNING: Exception occurred in plotting mv_polytopes-1 [dochtml] [combinat ] from /home/release/Sage/src/doc/en/reference/combinat/sage/combinat/crystals/mv_polytopes.rst: [dochtml] [combinat ] Traceback (most recent call last): [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/site-packages/matplotlib/sphinxext/plot_directive.py", line 472, in run_code [dochtml] [combinat ] exec(code, ns) [dochtml] [combinat ] File "<string>", line 4, in <module> [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/site-packages/sage/combinat/root_system/root_lattice_realizations.py", line 2053, in plot [dochtml] [combinat ] G += self.plot_roots(roots, plot_options=plot_options) [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/site-packages/sage/combinat/root_system/root_lattice_realizations.py", line 2299, in plot_roots [dochtml] [combinat ] roots = Family(roots, self) [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/site-packages/sage/sets/family.py", line 406, in Family [dochtml] [combinat ] return LazyFamily(indices, function, name) [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/site-packages/sage/sets/family.py", line 908, in __init__ [dochtml] [combinat ] self.set = copy(set) [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/copy.py", line 102, in copy [dochtml] [combinat ] return _reconstruct(x, None, *rv) [dochtml] [combinat ] File "/home/release/Sage/local/lib/python3.8/copy.py", line 264, in _reconstruct [dochtml] [combinat ] y = func(*args) [dochtml] [combinat ] File "stringsource", line 7, in sage.structure.sage_object.__pyx_unpickle_SageObject (build/cythonized/sage/structure/sage_object.c:11737) [dochtml] [combinat ] TypeError: sage.structure.sage_object.SageObject.__new__(FiniteFamily_with_category) is not safe, use FiniteFamily_with_category.__new__() [dochtml] [combinat ] /home/release/Sage/local/lib/python3.8/site-packages/sage/combinat/crystals/mv_polytopes.py:docstring of sage.combinat.crystals.mv_polytopes.MVPolytopes:101: WARNING: Exception occurred in plotting mv_polytopes-2
comment:9 Changed 2 years ago by
Thanks for testing, but I don't see how this error TypeError: sage.structure.sage_object.SageObject.__new__(FiniteFamily_with_category) is not safe, use FiniteFamily_with_category.__new__()
is related to the changes in this branch. Do you have any suggestions on how to investigate this?
comment:10 Changed 2 years ago by
Something very subtle is likely happening if it is indeed related to this branch. The first thing to do would be to try and reproduce the error. You might want to try with Volker's current branch to see if it is coming from one of the already merged-but-not-released branches for the upcoming 9.3.beta0. However, I am at a loss as well for the error and reproducing it.
comment:11 Changed 22 months ago by
Keywords: | sd111 added |
---|
Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111
comment:12 Changed 20 months ago by
Milestone: | sage-9.3 → sage-9.4 |
---|
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:13 Changed 15 months ago by
Milestone: | sage-9.4 → sage-9.5 |
---|
Setting a new milestone for this ticket based on a cursory review.
comment:14 Changed 9 months ago by
Milestone: | sage-9.5 → sage-9.6 |
---|
comment:15 Changed 6 months ago by
Milestone: | sage-9.6 → sage-9.7 |
---|
comment:16 Changed 4 weeks ago by
Milestone: | sage-9.7 → sage-9.8 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
Narrow down import of SageObject