Opened 14 months ago

Closed 7 months ago

#26526 closed defect (fixed)

mutable poset: remove default for breaking ties in topological sort

Reported by: dkrenn Owned by:
Priority: major Milestone: sage-8.8
Component: asymptotic expansions Keywords:
Cc: behackl Merged in:
Authors: Daniel Krenn Reviewers: Benjamin Hackl
Report Upstream: N/A Work issues:
Branch: fd98fc1 (Commits) Commit: fd98fc1fe5929efe1f9d84bd2c310d1d4a0cd63f
Dependencies: Stopgaps:

Description

At the moment shells_topological of a mutable poset breaks ties by means of repr by default. However, mutable poset is a data structure and should not do this expensive operation by default, therefore this default should be removed which is the aim of this ticket.

Note that this default was in mainly due to convenience reasons to have the results in the doctests always in the same order.

(Motivation: For my code/application, this improves the computation time by more than a factor 2.)

Change History (6)

comment:1 Changed 14 months ago by dkrenn

  • Branch set to u/dkrenn/repr-default-topological

comment:2 Changed 14 months ago by dkrenn

  • Authors set to Daniel Krenn
  • Cc behackl added
  • Commit set to fd98fc1fe5929efe1f9d84bd2c310d1d4a0cd63f
  • Status changed from new to needs_review

New commits:

fd98fc1Trac #26526: remove default "repr" for topological sorting

comment:3 follow-up: Changed 8 months ago by behackl

  • Milestone changed from sage-8.5 to sage-8.8
  • Reviewers set to Benjamin Hackl
  • Status changed from needs_review to needs_info

I looked through the changes and they look good to me overall.

Would it make sense to add another doctest (possibly with output marked as random) where no key is passed and thus no sorting happens?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 7 months ago by dkrenn

Replying to behackl:

Would it make sense to add another doctest (possibly with output marked as random) where no key is passed and thus no sorting happens?

I do not see much of a point adding such a test.

comment:5 in reply to: ↑ 4 Changed 7 months ago by behackl

  • Status changed from needs_info to positive_review

Replying to dkrenn:

Replying to behackl:

Would it make sense to add another doctest (possibly with output marked as random) where no key is passed and thus no sorting happens?

I do not see much of a point adding such a test.

I was thinking about testing whether calling the method without a key still works -- but you are right, I also cannot think of a usecase for that.

Good to go.

comment:6 Changed 7 months ago by vbraun

  • Branch changed from u/dkrenn/repr-default-topological to fd98fc1fe5929efe1f9d84bd2c310d1d4a0cd63f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.