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: | sage-7.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 follow-up: ↓ 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