Opened 5 years ago

Closed 4 years ago

#19290 closed defect (wontfix)

MatroidUnion _rank accept any iterator, partition accept empty partition

Reported by: chaoxu Owned by:
Priority: trivial Milestone: sage-duplicate/invalid/wontfix
Component: matroid theory Keywords:
Cc: Stefan, yomcat, Rudi Merged in:
Authors: Chao Xu Reviewers:
Report Upstream: N/A Work issues:
Branch: u/chaoxu/matroid_union (Commits) Commit: 4f501fbc5cdc0958d148bfe98b42a3a571a337e5
Dependencies: Stopgaps:

Description

Currently, in MatroidUnion? _rank(X) can only accept objects X that we can take the set difference with another frozenset. There are objects having frozenset interface, but cannot be used in taking differences.

The PartitionMatroid? can't handle empty partition.

Change History (6)

comment:1 Changed 5 years ago by chaoxu

  • Branch set to u/chaoxu/matroid_union

comment:2 Changed 5 years ago by chaoxu

  • Commit set to 4f501fbc5cdc0958d148bfe98b42a3a571a337e5
  • Status changed from new to needs_review

New commits:

4f501fbfix bugs: empty partition. _rank in MatroidUnion accepts any iterator.

comment:3 Changed 5 years ago by Rudi

  • Cc Rudi added
  • Component changed from PLEASE CHANGE to matroid theory

Hi Chao,

It is OK if underscored methods like ._rank are picky about the kind of input they can handle. The idea behind having both .rank() and ._rank() is that .rank() should be convenient to use (by checking the input, possibly reformatting it), and ._rank() as efficient as possible (by assuming the input is of the right form).

So unless there is no efficiency hit whatsoever, I don't like the idea of adjusting ._rank to accept more varied input.

comment:4 Changed 5 years ago by Stefan

  • Status changed from needs_review to needs_work

See http://doc.sagemath.org/html/en/reference/matroids/sage/matroids/matroid.html#sage.matroids.matroid.Matroid for an explanation of what we normally assume about inputs to our underscored functions. I agree with Rudi in this case, it is the responsibility of the caller to provide something with set interface to _rank, not the responsibility of _rank.

In addition, please keep your tickets to one issue per ticket: the PartitionMatroid? fix is fine, but can't go in because of the first issue.

comment:5 Changed 4 years ago by Stefan

  • Milestone changed from sage-6.9 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to positive_review

The first issue is not a bug; the second issue is now #23213

comment:6 Changed 4 years ago by embray

  • Resolution set to wontfix
  • Status changed from positive_review to closed

Closing tickets in the sage-duplicate/invalid/wontfix module with positive_review (i.e. someone has confirmed they should be closed).

Note: See TracTickets for help on using tickets.