Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#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 nthiery)

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)

trac_7421-register_coercion_weaker_assertion.patch​ (2.9 KB) - added by nthiery 8 years ago.
The previous version was missing a patch header.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by nthiery

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by mhansen

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.

comment:3 Changed 8 years ago by was

  • Milestone changed from sage-4.3 to sage-4.2.1

comment:4 follow-up: Changed 8 years ago by 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.

comment:5 Changed 8 years ago by nthiery

  • Description modified (diff)

comment:6 Changed 8 years ago by nthiery

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 8 years ago by nthiery

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?

Changed 8 years ago by nthiery

The previous version was missing a patch header.

comment:8 Changed 8 years ago by mhansen

  • 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 8 years ago by robertwb

  • Status changed from needs_review to positive_review

Yes, positive review.

comment:10 Changed 8 years ago by mhansen

  • 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 mvngu

  • Report Upstream set to N/A
  • Summary changed from Weaker precondition for registering a new coercion. to Weaker precondition for registering a new coercion
Note: See TracTickets for help on using tickets.