Opened 6 years ago
Closed 5 years ago
#21895 closed defect (fixed)
Better metaclass inference in dynamic classes
Reported by:  saraedum  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  categories  Keywords:  metaclass 
Cc:  nthiery, roed  Merged in:  
Authors:  Julian Rüth  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  86b4868 (Commits, GitHub, GitLab)  Commit:  86b4868e9deb41940e3bed2a3289276bd7557a52 
Dependencies:  Stopgaps: 
Description
When creating a dynamic class, (e.g., when creating an object in a category,) the following code is used to figure out the metaclass:
metaclass = DynamicMetaclass # The metaclass of a class must derive from the metaclasses of its # bases. The following handles the case where one of the base # classes is a known Sage metaclass. This approach won't scale # well if we start using metaclasses seriously in Sage. for base in bases: if isinstance(base, InheritComparisonClasscallMetaclass): metaclass = DynamicInheritComparisonClasscallMetaclass elif isinstance(base, ClasscallMetaclass): metaclass = DynamicClasscallMetaclass elif isinstance(base, InheritComparisonMetaclass): metaclass = DynamicInheritComparisonMetaclass
This does not work if two bases have conflicting metaclasses, say InheritComparisonMetaclass
and ClasscallMetaclass
. As a result, one can currently not let a subclass of Morphism
(which is a InheritComparisonMetaclass
) inherit from UniqueRepresentation
(a ClasscallMetaclass
).
Change History (15)
comment:1 Changed 6 years ago by
 Branch set to u/saraedum/better_metaclass_inference_in_dynamic_classes
comment:2 followup: ↓ 6 Changed 6 years ago by
 Commit set to c84f7ad60a8d8940e10109fc64a57662a59a8517
comment:3 Changed 6 years ago by
 Cc nthiery roed added
comment:4 Changed 6 years ago by
nthiery: I believe that you wrote the code that I quote in the summary?
comment:5 Changed 6 years ago by
 Status changed from new to needs_review
comment:6 in reply to: ↑ 2 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:7 Changed 6 years ago by
And I would also recommend to split off the changes which are unrelated to the topic of this ticket (the changes to AlgebraMorphism
for example).
comment:8 Changed 5 years ago by
 Commit changed from c84f7ad60a8d8940e10109fc64a57662a59a8517 to 0f93b46ad07600c3e268644025261647087e9029
Branch pushed to git repo; I updated commit sha1. New commits:
0f93b46  Add a doctest for 21895

comment:9 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:10 Changed 5 years ago by
 Status changed from needs_review to needs_work
 Compare classes with
is
instead of==
.
 Move the changes to
generic_basis_code.py
to a new ticket.
comment:11 Changed 5 years ago by
 Commit changed from 0f93b46ad07600c3e268644025261647087e9029 to 86b4868e9deb41940e3bed2a3289276bd7557a52
comment:12 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:13 Changed 5 years ago by
I dropped the other change since I do not care for that particular case.
comment:14 Changed 5 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:15 Changed 5 years ago by
 Branch changed from u/saraedum/better_metaclass_inference_in_dynamic_classes to 86b4868e9deb41940e3bed2a3289276bd7557a52
 Resolution set to fixed
 Status changed from positive_review to closed
I fixed a real world problem that hit me when working on #21879 to showcase that this really works. I could also split this off into a different ticket and create a more artificial doctest for this, if the reviewer wants me to.
New commits:
Better inference of metaclass when creating dynamic classes
Implement a morphism that inherits from UniqueRepresentation
fix typo
Do not skip test
Verify that unique representation works
Normalize arguments of AlgebraMorphism