#7157 closed enhancement (fixed)
neighbors_out/in instead of predecessor/successor in DiGraph
| Reported by: | ncohen | Owned by: | rlm |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.3 |
| Component: | graph theory | Keywords: | |
| Cc: | jason, rlm, nthiery | Merged in: | sage-4.3.alpha1 |
| Authors: | Nathann Cohen | Reviewers: | Florent Hivert, Mike Hansen |
| Report Upstream: | N/A | Work issues: | |
| Branch: | Commit: | ||
| Dependencies: | Stopgaps: |
Description (last modified by )
This patch replaces the names successors/iterators by neighbors_in and neighbors_out.
THIS PATCH DEPENDS ON #7515 !!!!
Attachments (2)
Change History (15)
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- 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: ↓ 4 Changed 10 years ago by
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 10 years ago by
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 10 years ago by
- 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 10 years ago by
- 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 10 years ago by
Thank you very much !!! :-)
Changed 10 years ago by
comment:9 in reply to: ↑ 8 Changed 10 years ago by
Replying to ncohen:
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 ;-)
comment:10 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_review to positive_review
Changed 10 years ago by
comment:11 Changed 10 years ago by
- Merged in set to sage-4.3.alpha1
- Resolution set to fixed
- Reviewers set to Florent Hivert, Mike Hansen
- Status changed from positive_review to closed
I've added a new trac_7157_review.patch patch with two function calls that were missed.
comment:12 Changed 10 years ago by
Thank you ! :-)
comment:13 Changed 10 years ago by
- Summary changed from [with patch, needs review] neighbors_out/in instead of predecessor/successor in DiGraph to neighbors_out/in instead of predecessor/successor in DiGraph
On my side, it passes -testall without any (related) failure.