Opened 11 years ago

Last modified 11 years ago

#7157 closed enhancement

[with patch, needs review] neighbors_out/in instead of predecessor/successor in DiGraph — at Version 8

Reported by: ncohen Owned by: rlm
Priority: major Milestone: sage-4.3
Component: graph theory Keywords:
Cc: jason, rlm, nthiery Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by ncohen)

This patch replaces the names successors/iterators by neighbors_in and neighbors_out.

This patch requires #7515 to be applied first

Change History (9)

comment:1 Changed 11 years ago by ncohen

  • Status changed from new to needs_review

On my side, it passes -testall without any (related) failure.

comment:2 Changed 11 years ago by rlm

  • Status changed from needs_review to needs_work
  • Summary changed from [with patch, needs review] neighbors_out/in instead of predecessor/successor in DiGraph to [with patch, needs work] neighbors_out/in instead of predecessor/successor in DiGraph

You shouldn't just remove the successor/predecessor terminology. A lot of people (e.g. Chris Godsil) might have to do a lot of work to change all their personal scripts etc. to reflect this change. This patch breaks backwards compatibility.

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

Should I just add aliases and let the old functions exist ? Should we keep two copies of the same functions ?

comment:4 in reply to: ↑ 3 Changed 11 years ago by rlm

Replying to ncohen:

Should I just add aliases and let the old functions exist ? Should we keep two copies of the same functions ?

I think aliases would be okay, but people have mentioned before that aliases are bad. Please bring this up on the sage-devel thread, and do what the consensus is there.

comment:5 Changed 11 years ago by ncohen

  • Status changed from needs_work to needs_review
  • Summary changed from [with patch, needs work] neighbors_out/in instead of predecessor/successor in DiGraph to [with patch, needs review] neighbors_out/in instead of predecessor/successor in DiGraph

Should be better now :-)

The new functions are defined, old functions are marked "Deprecated".

comment:6 Changed 11 years ago by hivert

  • Report Upstream set to N/A

Hi there,

Just a short remark: If you wan't to shorten the patch: see #7515.

comment:7 Changed 11 years ago by ncohen

Thank you very much !!! :-)

comment:8 Changed 11 years ago by ncohen

  • Description modified (diff)

Updated !

Changed 11 years ago by ncohen

Note: See TracTickets for help on using tickets.