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:

Status badges

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 git

Commit: ff9ed515c45cd8a66a6b2cf9f96b8604d6ae24fe

Branch pushed to git repo; I updated commit sha1. New commits:

ff9ed51Narrow down import of SageObject

comment:2 Changed 2 years ago by Tobias Diez

Status: newneeds_review

New commits:

ff9ed51Narrow down import of SageObject

New commits:

ff9ed51Narrow down import of SageObject

comment:3 Changed 2 years ago by Matthias Köppe

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 in reply to:  3 Changed 2 years ago by Travis Scrimshaw

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 Matthias Köppe

Reviewers: Travis ScrimshawTravis Scrimshaw, Matthias Koeppe
Status: needs_reviewpositive_review

comment:6 Changed 2 years ago by Tobias Diez

Thanks for the review!

comment:7 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:8 Changed 2 years ago by Volker Braun

Status: positive_reviewneeds_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 Tobias Diez

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 Travis Scrimshaw

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 Matthias Köppe

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 Matthias Köppe

Milestone: sage-9.3sage-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 Matthias Köppe

Milestone: sage-9.4sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:14 Changed 9 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:15 Changed 6 months ago by Matthias Köppe

Milestone: sage-9.6sage-9.7

comment:16 Changed 4 weeks ago by Matthias Köppe

Milestone: sage-9.7sage-9.8
Note: See TracTickets for help on using tickets.