Opened 7 years ago
Closed 7 years ago
#15466 closed defect (fixed)
Remove deprecated code from combinat/
Reported by: | ncohen | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.2 |
Component: | combinatorics | Keywords: | |
Cc: | sage-combinat | Merged in: | |
Authors: | Nathann Cohen | Reviewers: | Andrew Mathas, Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | d5086d0 (Commits) | Commit: | d5086d01765e15f320a9399faf5ce521ddf12eaf |
Dependencies: | #15467 | Stopgaps: |
Change History (23)
comment:1 Changed 7 years ago by
- Branch set to u/ncohen/15466
- Cc sage-combinat added
- Dependencies set to #15467
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Remove deprecated code from combinat.py to Remove deprecated code from combinat/
comment:2 Changed 7 years ago by
- Commit set to a8bd31096c7f93c1c1a51ac03b634d85a07a0dd7
comment:3 Changed 7 years ago by
- Branch changed from u/ncohen/15466 to u/andrew.mathas/ticket/15466
- Created changed from 11/29/13 14:11:00 to 11/29/13 14:11:00
- Modified changed from 11/29/13 19:24:58 to 11/29/13 19:24:58
comment:4 follow-up: ↓ 6 Changed 7 years ago by
- Commit changed from a8bd31096c7f93c1c1a51ac03b634d85a07a0dd7 to 6639e7543dc6399280c5e42babca5eff18584da9
- Milestone changed from sage-5.13 to sage-6.0
- Reviewers set to Andrew Mathas
Hi Nathann, thanks for starting this (or should that be thaaaaaaaaaaaanks!:).
I found a few more depreciated [sic] functions to remove in the words subdirectory and I moved a few import statements of deprecation
so that they are now immediately above the function call. This will help ensure that there are no stray deprecation
import statements left when the corresponding soon-to-be-depreciated code is removed. Finally, I killed off some documentation for some functions that are removed by this patch.
If you are happy with my changes let's make this a positive review. I've bumped the version number to 6 since we're using git.
[ps I have checked all of the doctsts in sage/combinat. I do get a doctext failure in root_system/coxeter_group.py
but I get the same failure in master, so I think it has nothing to do with this ticket.]
New commits:
6639e75 | A few more deprecations |
comment:5 Changed 7 years ago by
- Description modified (diff)
comment:6 in reply to: ↑ 4 Changed 7 years ago by
- Status changed from needs_review to positive_review
Yooooooooooo !!
Hi Nathann, thanks for starting this (or should that be thaaaaaaaaaaaanks!:).
Well I'm glad to receive your help with this ! :-D
I found a few more depreciated [sic] functions to remove in the words subdirectory and I moved a few import statements of
deprecation
so that they are now immediately above the function call. This will help ensure that there are no straydeprecation
import statements left when the corresponding soon-to-be-depreciated code is removed.
Finally, I killed off some documentation for some functions that are removed by this patch.
Argggg ! Nice job spotting that ;-)
By the way : these days I use the --warn-links flag when building the doc. It tells you whenever some links are made toward non-existent functions. I guess it will be useful to spot things like that in the future, when this --warn-links will be a default, though right now there are too many broken lins in Sage to make it the default :-)
If you are happy with my changes let's make this a positive review. I've bumped the version number to 6 since we're using git.
The tests passed while I was reading the patch. Good to go, thanks :-)
[ps I have checked all of the doctsts in sage/combinat. I do get a doctext failure in
root_system/coxeter_group.py
but I get the same failure in master, so I think it has nothing to do with this ticket.]
Hmmmmm.. I don't get this error. Though at some point Volker told me that I should run "make" again in Sage's directory to update the spkg which may have changed since. Do give it a try when you don't need to use Sage for a while (it will recompile a lot of things). I used to have a couple of broken doctests, and it solved it !
Thank you again, and positive review to this patch ! ;-)
Nathann
comment:7 Changed 7 years ago by
- Milestone changed from sage-6.0 to sage-6.1
comment:8 Changed 7 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
comment:9 Changed 7 years ago by
- Resolution fixed deleted
- Status changed from closed to new
sage -t src/sage/misc/cachefunc.pyx ********************************************************************** File "src/sage/misc/cachefunc.pyx", line 534, in sage.misc.cachefunc.CachedFunction.__init__ Failed example: g.cache Expected: {((5, None, 'default'), ()): 7} Got: {((5, 'default'), ()): 7} ********************************************************************** File "src/sage/misc/cachefunc.pyx", line 722, in sage.misc.cachefunc.CachedFunction.__call__ Failed example: g.get_cache() Expected: {((5, None, 'default'), ()): 7} Got: {((5, 'default'), ()): 7} ********************************************************************** File "src/sage/misc/cachefunc.pyx", line 761, in sage.misc.cachefunc.CachedFunction.get_cache Failed example: g.get_cache() Expected: {((5, None, 'default'), ()): 7} Got: {((5, 'default'), ()): 7} ********************************************************************** File "src/sage/misc/cachefunc.pyx", line 802, in sage.misc.cachefunc.CachedFunction.set_cache Failed example: g.get_cache() Expected: {((5, None, 'default'), ()): 7} Got: {((5, 'default'), ()): 7} ********************************************************************** File "src/sage/misc/cachefunc.pyx", line 805, in sage.misc.cachefunc.CachedFunction.set_cache Failed example: g.get_cache() Expected: {((5, None, 'default'), ()): 17} Got: {((5, 'default'), ()): 17} ********************************************************************** File "src/sage/misc/cachefunc.pyx", line 867, in sage.misc.cachefunc.CachedFunction.clear_cache Failed example: g.get_cache() Expected: {((5, None, 'default'), ()): 7} Got: {((5, 'default'), ()): 7} **********************************************************************
comment:10 Changed 7 years ago by
- Branch changed from u/andrew.mathas/ticket/15466 to u/tscrim/ticket/15466
- Commit changed from 6639e7543dc6399280c5e42babca5eff18584da9 to cd43d7b88cd56b54165a8d5c9b19a0e4ed796bb7
- Status changed from new to needs_review
comment:11 Changed 7 years ago by
- Status changed from needs_review to positive_review
I didn't understand why this patch should affect cachefunc.pyx
but the doc-tests causing the problems all call number_of_partitions
. Part of this patch removes depreciated code in partition.py
and, in particular, it changes the calling syntax by removing a depreciated argument of k=None
. This both explains the errors above and says that the correct fix is simply to change the output of the doct-tests as the patch does. Hence, positive review.
Andrew
comment:12 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:13 Changed 7 years ago by
conflict, please merge in latest beta
comment:14 Changed 7 years ago by
- Commit changed from cd43d7b88cd56b54165a8d5c9b19a0e4ed796bb7 to 25488e8974f3d986b5be719e5aaa61d9910becb8
- Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
25488e8 | Merge branch 'u/tscrim/ticket/15466' of trac.sagemath.org:sage into u/tscrim/ticket/15466
|
comment:15 Changed 7 years ago by
- Reviewers changed from Andrew Mathas to Andrew Mathas, Travis Scrimshaw
- Status changed from needs_review to positive_review
Done.
comment:16 Changed 7 years ago by
Documentation doesn't build
build/pipestatus "./sage --docbuild --no-pdf-links all html -j 2>&1" "tee -a logs/dochtml.log" Setting permissions of DOT_SAGE directory so only you can read and write it. Traceback (most recent call last): File "/Users/buildslave-sage/slave/sage_git/build/src/doc/common/builder.py", line 1465, in <module> import sage.all File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/all.py", line 133, in <module> from sage.combinat.all import * File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/combinat/all.py", line 1, in <module> from combinat import bell_number, catalan_number, euler_number, fibonacci, \ ImportError: cannot import name combinations
comment:17 Changed 7 years ago by
I'm unable to reproduce this. I ran sync-build, nuked my docbuild, and rebuilt the entire thing.
comment:18 Changed 7 years ago by
Might be due to #9505, please wait until 6.2.beta3 is out and then merge that in.
comment:19 Changed 7 years ago by
6.2.beta3 is out ;-)
comment:20 Changed 7 years ago by
- Branch changed from u/tscrim/ticket/15466 to u/ncohen/15466
- Commit changed from 25488e8974f3d986b5be719e5aaa61d9910becb8 to d5086d01765e15f320a9399faf5ce521ddf12eaf
comment:21 Changed 7 years ago by
Hmmm...strange... I didn't see those imports when looking at the branch before. *shrugs*
PS - Thanks Nathann.
comment:22 Changed 7 years ago by
Yeah, I find it strange too.... And I don't see why they could have been added from another patch in the meantime O_o
Nathann
comment:23 Changed 7 years ago by
- Branch changed from u/ncohen/15466 to d5086d01765e15f320a9399faf5ce521ddf12eaf
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits: