Opened 3 years ago

Closed 3 years ago

#27819 closed defect (fixed)

Poset(), error checking when linear_extension=True

Reported by: jmantysalo Owned by:
Priority: minor Milestone: sage-8.9
Component: combinatorics Keywords: days100
Cc: chapoton Merged in:
Authors: Julian Ritter Reviewers: Jori Mäntysalo
Report Upstream: N/A Work issues:
Branch: 167a90a (Commits, GitHub, GitLab) Commit: 167a90a76cb3425f53691e4011b7e5103faac776
Dependencies: Stopgaps:

Status badges

Description

sage: Poset(( [1,2,3,3], [[1,2]])).list()
[3, 1, 2]
sage: Poset(( [1,2,3,3], [[1,2]]), linear_extension=True).list()
[1, 2, 3, 3]

Change History (15)

comment:1 Changed 3 years ago by embray

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

comment:2 Changed 3 years ago by embray

  • Milestone sage-8.8 deleted

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

comment:3 Changed 3 years ago by nailuj

  • Branch set to u/nailuj/poset____error_checking_when_linear_extension_true

comment:4 Changed 3 years ago by nailuj

  • Authors set to Julian Ritter
  • Cc chapoton added
  • Commit set to 0a971ffebdb893888f446e6d3044c380186a5d95
  • Keywords days100 added
  • Status changed from new to needs_info

I added a sanity check for duplicate elements if linear_extension is set to True. Maybe such a sanity check should also be performed if linear_extension is False?

I propose to throw a ValueError if there are duplicates. Alternatively, it would be possible to clean up the list of elements instead of throwing an error.


New commits:

0a971ffadded check for duplicate elements if linear_extension=True

comment:5 Changed 3 years ago by jmantysalo

I suppose that users can do something like Poset(( f.roots()+g.roots(), lamda r1, r2: ... )) and so we should allow duplicates in the element list if linear_extension=False.

I think this patch can go, modulo normal bikeshedding: exception message should not begin with a capital letter. Maybe it should even mention linear extension, i.e. "input contains duplicate elements and linear_extension=True".

comment:6 Changed 3 years ago by git

  • Commit changed from 0a971ffebdb893888f446e6d3044c380186a5d95 to 21d360516264c681851ee5ddac7aba361361e25e

Branch pushed to git repo; I updated commit sha1. New commits:

21d3605more precise error message

comment:7 Changed 3 years ago by git

  • Commit changed from 21d360516264c681851ee5ddac7aba361361e25e to eb7512e0b98d970b037ebfe8145fb8c2ee5a1e99

Branch pushed to git repo; I updated commit sha1. New commits:

eb7512ecorrect line break in error message

comment:8 follow-up: Changed 3 years ago by chapoton

you need to add doctests to display what you have fixed

comment:9 Changed 3 years ago by git

  • Commit changed from eb7512e0b98d970b037ebfe8145fb8c2ee5a1e99 to 167a90a76cb3425f53691e4011b7e5103faac776

Branch pushed to git repo; I updated commit sha1. New commits:

167a90aadded doctest

comment:10 in reply to: ↑ 8 Changed 3 years ago by nailuj

Replying to chapoton:

you need to add doctests to display what you have fixed

Done.

comment:11 follow-up: Changed 3 years ago by jmantysalo

I think this would be good to go. But you still have this in needs_info, are you planning to do more?

comment:12 in reply to: ↑ 11 Changed 3 years ago by nailuj

  • Status changed from needs_info to needs_review

Replying to jmantysalo:

I think this would be good to go. But you still have this in needs_info, are you planning to do more?

No, I just forgot to change the status. Thanks for the reminder.

comment:13 Changed 3 years ago by jmantysalo

  • Reviewers set to Jori Mäntysalo
  • Status changed from needs_review to positive_review

OK then.

comment:14 Changed 3 years ago by chapoton

  • Milestone set to sage-8.9

comment:15 Changed 3 years ago by vbraun

  • Branch changed from u/nailuj/poset____error_checking_when_linear_extension_true to 167a90a76cb3425f53691e4011b7e5103faac776
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.