#7421 closed enhancement (fixed)
Weaker precondition for registering a new coercion
Reported by: | nthiery | Owned by: | robertwb |
---|---|---|---|
Priority: | blocker | Milestone: | sage-4.3 |
Component: | coercion | Keywords: | coercion |
Cc: | sage-combinat, robertwb | Merged in: | sage-4.3.alpha0 |
Authors: | Nicolas M. Thiéry | Reviewers: | Robert Bradshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
With the attached patch, the precondition for registering a new coercion from P to Q with register_coercion becomes:
"no coercion into P has been queried, or no coercion from P to Q has been registered or discovered earlier"
Which is a bit weaker than the previous:
"no coercion into P has been queried"
This should still be quite safe, while covering all the formerly problematic practical use cases coming up in the category code #5981.
Attachments (1)
Change History (12)
comment:1 Changed 7 years ago by
- Status changed from new to needs_review
comment:2 Changed 7 years ago by
comment:3 Changed 7 years ago by
- Milestone changed from sage-4.3 to sage-4.2.1
comment:4 follow-up: ↓ 7 Changed 7 years ago by
There is one way this can go wrong. Suppose one has B -> C, and one wants to register A -> B. If A -> C was previously requested, its non-existence will be cached.
Now I don't think this will be an issue in practice, nor does _unset_coercions_used
solve it.
comment:5 Changed 7 years ago by
- Description modified (diff)
comment:6 Changed 7 years ago by
With the updated patch register_coercion also does not bark if _coercions_used is false.
Otherwise, this triggered to failing doctests in sage/modules/fg_pid/fgp_module.py and sage/modules/fg_pid/fgp_morphism.py. An expert should investigate why a coercion gets registered twice in those modules, and whether this is not a bug. But I vote for postponing that for later.
comment:7 in reply to: ↑ 4 Changed 7 years ago by
Replying to robertwb:
There is one way this can go wrong. Suppose one has B -> C, and one wants to register A -> B. If A -> C was previously requested, its non-existence will be cached.
Now I don't think this will be an issue in practice, nor does
_unset_coercions_used
solve it.
I agree.
So, is this positive review?
comment:8 Changed 7 years ago by
- Milestone changed from sage-4.2.1 to sage-4.3
I'm going to move this to 4.3 where it's more relevant.
comment:9 Changed 7 years ago by
- Status changed from needs_review to positive_review
Yes, positive review.
comment:10 Changed 7 years ago by
- Merged in set to sage-4.3.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:11 Changed 7 years ago by
- Report Upstream set to N/A
- Summary changed from Weaker precondition for registering a new coercion. to Weaker precondition for registering a new coercion
I think is probably okay. After thinking about it for a bit, I could come up with a situation where this change would make the coercion graph non-commutatitve.
I'll run all the tests with it here in a bit.