Opened 15 years ago

Closed 15 years ago

#749 closed task (fixed)

[with patch, is fixed] graphs: enum() functionality duplicated in relabel()

Reported by: jason Owned by: rlm
Priority: major Milestone: sage-2.8.10
Component: combinatorics Keywords: graphs
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The enum() code is duplicated in relabel() for the quick option. It sure would be nice to factor that out so that the enum() code was all in one place.

Attachments (2)

trac749_1.patch (2.1 KB) - added by rlm 15 years ago.
This one first, removed quick option from relabel().
trac749_2.patch (14.1 KB) - added by rlm 15 years ago.
This is the main overhaul.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 15 years ago by rlm

See ticket #729 for some relevant comments...

comment:2 Changed 15 years ago by rlm

Or instead, just read them here:

cmp ultimateley ends up using enum() twice, and this is kind of too much, since we could just look at the two graphs from lexicographically most significant, and return the answer as soon as they differ. Also, as Jason points out we should be using eq, neq, lt, le, gt, ge instead.

comment:3 Changed 15 years ago by rlm

Note - the quick option in the relabel function was originally an optimization for the graph_isom code, and no longer applies, since it is never used. Other than that purpose, it doesn't make sense to have it as an option, so I'll just remove it.

Here is another aspect:

sage: G._nxg.adj
{0: {1: [None]}, 1: {0: [None]}}
sage: H._nxg.adj
{0: {1: [None, None]}, 1: {0: [None, None]}}
sage: G == H
True

comment:4 Changed 15 years ago by rlm

  • Owner changed from was to rlm
  • Status changed from new to assigned
  • Summary changed from graphs: enum() functionality duplicated in relabel() to [with patch, is fixed] graphs: enum() functionality duplicated in relabel()

There is an attached patch that cleans up the whole situation.

Changed 15 years ago by rlm

This one first, removed quick option from relabel().

Changed 15 years ago by rlm

This is the main overhaul.

comment:5 Changed 15 years ago by rlm

  • Milestone changed from sage-2.9 to sage-2.8.10

comment:6 Changed 15 years ago by cwitty

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.