#14519: Cythonize ElementWrapper and make parent the first argument
-----------------------------------------+----------------------------------
Reporter: tscrim | Owner: tscrim
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.11
Component: performance | Resolution:
Keywords: cython ElementWrapper | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Travis Scrimshaw | Merged in:
Dependencies: #14143 #14015 #14516 | Stopgaps:
-----------------------------------------+----------------------------------
Changes (by tscrim):
* dependencies: #14143 #14516 => #14143 #14015 #14516
Comment:
Hey Nicolas,
Thank you for reviewing the patch.
Replying to [comment:6 nthiery]:
> Some points:
>
> - In those places where the patch touches a "..." continuation, use the
occasion to make it a "....:"
> - Line 312 of the coercion tutorial: missing space before D (it was
missing already)
> - In all the calls to MyElement: proper spacing between arguments
> - universal_cyclotomic_field.py:
> - Unless unavoidable, don't change the order of the imports, and keep
CombinatorialFreeModule imported in the __init__ instead of in the module.
> - Mention in the code that UCFElement can't multiple inherit from the
two extension types FieldElement and ElementWrapper, which is why at this
point the methods _latex_ and friends have to be duplicated (in the long
run we can hope not to need anymore to inherit from FieldElement).
Done.
> - Check the spacing between the arguments in the call to element_class
I think done; if not I'll need something more specific.
> - disjoint_union_enumerated_sets.py: why did the type of ``el`` change?
Because it because an extension class.
> - The __nonzero__ feature is new, right? I'd rather avoid it at this
point, since the semantic of being zero might differ quite some depending
on the use case.
Done.
> Let me know when you will have posted an updated patch fixing those as
well as the failing tests detected by the patchbot.
The updated patch fixes almost everything except 1 doctest in
`misc/nested_class_test.py` (as opposed to 2 previously) and I'm not quite
sure what the problem is currently. I don't think I need a `__reduce__()`
in the element wrapper since it has a `__dict__` (and my `__reduce__()`
was causing pickling to fail). If you have any insight into this, I'd
appreciate it.
Thanks,[[BR]]
Travis
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14519#comment:7>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
