Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#8402 closed defect (fixed)

Sanity check for Parents and Elements

Reported by: hivert Owned by: hivert
Priority: major Milestone: sage-4.4.1
Component: categories Keywords: Parent, Element, equality, zero, one
Cc: sage-combinat Merged in: sage-4.4.1.alpha2
Authors: Florent Hivert Reviewers: Nicolas M. Thiéry
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by hivert)

Here is the summary of what was decided on sage=devel:

1 - Any Parent or Element must have an equality methods such that self == self and self != None. This is not required for general SageObject. I also test that self != self and self == None both returns False.

2 - Element construction should be idempotent. More precisely, for any element e within parent P, the equality P(e) == e must hold. This should only be enforced for true parent since it cause some problems for facade parents.

Case by case exception such as RealIntervalField are possible.

3 - _test_hash: test that the result of hash is an int or that it raises an appropriate exception

3' - element of a parent in the category Monoid() (respectively CommutativeAdditiveMonoid()) must have a __hash__ method, which may raise an error for mutable element but never on .one() (respectively .zero())

Attachments (1)

trac_8402-categories_missing_test-fh.patch (56.1 KB) - added by hivert 11 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 11 years ago by hivert

  • Owner changed from nthiery to hivert

comment:2 Changed 11 years ago by hivert

  • Description modified (diff)
  • Summary changed from Sanity check for Parent and Elemet to Sanity check for Parents and Elements

I'm preparing some patches on sage-combinat queue. I already caught some bug with it: see #8695.

comment:3 Changed 11 years ago by nthiery

Review in process on the Sage-Combinat queue. All tests pass with sage-4.4-alpha0 (with the other patches under review applied as well)

comment:4 Changed 11 years ago by nthiery

  • Reviewers set to Nicolas M. Thiéry

comment:5 follow-up: Changed 11 years ago by nthiery

  • Cc sage-combinat added
  • Status changed from new to needs_review

All test still pass, even after my reviewer's patch on the patch queue :-)

Florent: please double check it. If that's fine, fold everything together, post here and set a positive review!

Note: I ended up throwing away a bit of code in crystals/spins.py, which was the easiest way to fix equality :-)

comment:6 in reply to: ↑ 5 Changed 11 years ago by hivert

  • Status changed from needs_review to positive_review

Florent: please double check it. If that's fine, fold everything together, post here and set a positive review!

All tests passed on sage, I folded and set positive review.

Changed 11 years ago by hivert

comment:7 Changed 11 years ago by was

  • Merged in set to 4.4.1.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:8 Changed 11 years ago by mvngu

  • Merged in changed from 4.4.1.alpha2 to sage-4.4.1.alpha2
Note: See TracTickets for help on using tickets.