Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

neighbors_out/in instead of predecessor/successor in DiGraph #7157

Closed
nathanncohen mannequin opened this issue Oct 8, 2009 · 16 comments
Closed

neighbors_out/in instead of predecessor/successor in DiGraph #7157

nathanncohen mannequin opened this issue Oct 8, 2009 · 16 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 8, 2009

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

THIS PATCH DEPENDS ON #7515 !!!!

CC: @jasongrout @rlmill @nthiery

Component: graph theory

Author: Nathann Cohen

Reviewer: Florent Hivert, Mike Hansen

Merged: sage-4.3.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/7157

@nathanncohen nathanncohen mannequin added this to the sage-4.3 milestone Oct 8, 2009
@nathanncohen nathanncohen mannequin assigned rlmill Oct 8, 2009
@nathanncohen nathanncohen mannequin added the s: needs review label Oct 8, 2009
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Oct 8, 2009

comment:1

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

@nathanncohen nathanncohen mannequin added the s: needs review label Oct 8, 2009
@rlmill
Copy link
Mannequin

rlmill mannequin commented Oct 8, 2009

comment:2

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.

@rlmill rlmill mannequin added s: needs work and removed s: needs review labels Oct 8, 2009
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Oct 8, 2009

comment:3

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

@rlmill
Copy link
Mannequin

rlmill mannequin commented Oct 8, 2009

comment:4

Replying to @nathanncohen:

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Oct 9, 2009

comment:5

Should be better now :-)

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

@hivert
Copy link

hivert commented Nov 22, 2009

comment:6

Hi there,

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Nov 22, 2009

comment:7

Thank you very much !!! :-)

@nathanncohen

This comment has been minimized.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Nov 24, 2009

comment:8

Updated !

@hivert
Copy link

hivert commented Nov 30, 2009

comment:9

Attachment: trac_7157.patch.gz

Replying to @nathanncohen:

Updated !

It was decided on sage-devel only to put the version and not the date in the message of deprecation aliases. I just uploaded a patch witch does that. Please review. Aside from that

You have a Positive review on trac_7157.patch. You can change the status as soon as you had an eye on my trivial review patch.

Cheers,

Florent

By the way a review on #7515 is welcome ;-)

@nathanncohen

This comment has been minimized.

@mwhansen
Copy link
Contributor

mwhansen commented Dec 1, 2009

Author: Nathann Cohen

@mwhansen
Copy link
Contributor

mwhansen commented Dec 1, 2009

Reviewer: Florent Hivert, Mike Hansen

@mwhansen
Copy link
Contributor

mwhansen commented Dec 1, 2009

Merged: sage-4.3.alpha1

@mwhansen
Copy link
Contributor

mwhansen commented Dec 1, 2009

comment:11

Attachment: trac_7157_review.patch.gz

I've added a new trac_7157_review.patch patch with two function calls that were missed.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Dec 1, 2009

comment:12

Thank you ! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants