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:  sage6.2 
Component:  combinatorics  Keywords:  
Cc:  sagecombinat  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Andrew Mathas, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  d5086d0 (Commits, GitHub, GitLab)  Commit:  d5086d01765e15f320a9399faf5ce521ddf12eaf 
Dependencies:  #15467  Stopgaps: 
Change History (23)
comment:1 Changed 7 years ago by
 Branch set to u/ncohen/15466
 Cc sagecombinat 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 followup: ↓ 6 Changed 7 years ago by
 Commit changed from a8bd31096c7f93c1c1a51ac03b634d85a07a0dd7 to 6639e7543dc6399280c5e42babca5eff18584da9
 Milestone changed from sage5.13 to sage6.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 soontobedepreciated 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 soontobedepreciated 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 warnlinks flag when building the doc. It tells you whenever some links are made toward nonexistent functions. I guess it will be useful to spot things like that in the future, when this warnlinks 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 sage6.0 to sage6.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 doctests 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 docttests as the patch does. Hence, positive review.
Andrew
comment:12 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.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 nopdflinks 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/buildslavesage/slave/sage_git/build/src/doc/common/builder.py", line 1465, in <module> import sage.all File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/all.py", line 133, in <module> from sage.combinat.all import * File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/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 rebuilt, ran syncbuild, rebuilt, nuked my docbuild, and rebuilt the entire doc.
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: