Opened 10 years ago

Last modified 9 years ago

#14287 closed enhancement

Split _test_elements_eq — at Version 14

Reported by: Julian Rüth Owned by: Nicolas M. Thiéry
Priority: trivial Milestone: sage-5.10
Component: categories Keywords:
Cc: David Roe Merged in:
Authors: Julian Rueth Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14284 Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

Currently _test_elements_eq tests for reflexivity, symmetry, and transitivity. However, transitivity does not hold in padic rings. This tickets splits _test_elements_eq into these three subtests, so they that we can override the transitivity test for padic implementations.


Apply

  1. trac_14287.patch
  2. 14287_review_58rc0.patch

Change History (14)

comment:1 Changed 10 years ago by Julian Rüth

Dependencies: #12484

comment:2 Changed 10 years ago by Julian Rüth

Dependencies: #12484#14284
Status: newneeds_review

comment:3 Changed 10 years ago by Julian Rüth

Description: modified (diff)

comment:4 Changed 10 years ago by Julian Rüth

apply trac_14287.patch 14287_review_58rc0.patch

Last edited 10 years ago by Julian Rüth (previous) (diff)

comment:5 Changed 10 years ago by Julian Rüth

Description: modified (diff)

comment:6 Changed 10 years ago by Julian Rüth

apply trac_14287.patch 14287_review_58rc0.patch

comment:7 Changed 10 years ago by Julian Rüth

apply trac_14287.patch 14287_review.patch

comment:8 Changed 10 years ago by Julian Rüth

Description: modified (diff)

comment:9 Changed 10 years ago by David Roe

Reviewers: David Roe
Status: needs_reviewpositive_review

Tests pass; looks good.

comment:10 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-5.9sage-pending

comment:11 Changed 10 years ago by Nicolas M. Thiéry

Just a micro comment about the use of this ticket; not its implementation.

However, transitivity does not hold in padic rings

Sounds scary! Well, as long as you know what you are doing :-)

so they that we can override the transitivity test for padic implementations

I am not sure what's your plan for this; just in case, and unless this becomes too cumbersome, I prefer writing explicitly each time:

    sage: TestSuite(...).run(skip="....transitivity")

rather than systematically disabling the transitivity test. First, that's the occasion to warn the user about an atypical behavior. And to get more people to thing about creative ways of fixing that. Second, the test might actually pass in certain cases, and it's a useful information to record.

Cheers,

Nicolas

comment:12 in reply to:  11 Changed 10 years ago by Julian Rüth

I see your point. However, these tests will not pass and there is no way to avoid this (other than disabling == completely). It's an intrinsic problem with padics (or anything that is stored to different precision).

I agree that we could use a skip to warn the user but on the other hand it is probably more confusing...

comment:13 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-pendingsage-5.9

comment:14 Changed 10 years ago by Jeroen Demeyer

Description: modified (diff)
Note: See TracTickets for help on using tickets.