Opened 4 years ago
Closed 4 years ago
#22796 closed enhancement (fixed)
Remove deprecated_callable_import
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  misc  Keywords:  
Cc:  vdelecroix  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  0ac7893 (Commits)  Commit:  0ac7893146826afa62ffe437868e8c5d6d313c17 
Dependencies:  Stopgaps: 
Description (last modified by )
Change History (24)
comment:1 Changed 4 years ago by
 Description modified (diff)
comment:2 Changed 4 years ago by
 Description modified (diff)
 Summary changed from Deprecate deprecated_callable_import to Remove deprecated_callable_import
comment:3 Changed 4 years ago by
 Branch set to u/jdemeyer/deprecate_deprecated_callable_import
comment:4 Changed 4 years ago by
 Commit set to 4b90c163b2d85c5fd6a661ef8308471ddf2cd6bd
comment:5 Changed 4 years ago by
 Cc vdelecroix added
 Status changed from new to needs_review
comment:6 followup: ↓ 7 Changed 4 years ago by
To make lazy_import
invisible, I think the following is cleaner
from sage.misc.lazy_import import lazy_import lazy_import('sage.some.package', 'some_function', deprecation=666) del lazy_import
comment:7 in reply to: ↑ 6 Changed 4 years ago by
Replying to vdelecroix:
I think the following is cleaner
I think that import lazy_import as _lazy_import
is better, because there are less chances for mistakes (one could easily forget to add the del lazy_import
or accidentally remove that statement).
comment:8 Changed 4 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
This looks good to me.
comment:9 Changed 4 years ago by
 Status changed from positive_review to needs_work
Merge conflict...
comment:10 Changed 4 years ago by
it does merge cleanly in 8.0.beta5, thus I have no idea where the merge conflict comes from.
comment:11 Changed 4 years ago by
 Status changed from needs_work to positive_review
comment:12 Changed 4 years ago by
 Status changed from positive_review to needs_work
Merge conflict... Obviously the merge conflict comes from stuff in beta6, duh.
comment:13 Changed 4 years ago by
 Branch changed from u/jdemeyer/deprecate_deprecated_callable_import to u/dimpase/t22796
 Commit changed from 4b90c163b2d85c5fd6a661ef8308471ddf2cd6bd to 18e9a467945579b5fc907de02a46bef4eb888f0f
 Status changed from needs_work to positive_review
it was weird seeing "Merge conflict" against a branch that was not public yet :) anyhow, it's a trivial merge, one chunk was to be deleted.
comment:14 Changed 4 years ago by
 Status changed from positive_review to needs_work
sage t long warnlong 72.0 src/sage/combinat/designs/design_catalog.py ********************************************************************** File "src/sage/combinat/designs/design_catalog.py", line 77, in sage.combinat.designs.design_catalog Failed example: 'absolute_import' in dir(designs) or 'deprecated_callable_import' in dir(designs) Expected: False Got: True ********************************************************************** 1 item had failures: 1 of 3 in sage.combinat.designs.design_catalog [2 tests, 1 failure, 0.27 s]  sage t long warnlong 72.0 src/sage/combinat/designs/design_catalog.py # 1 doctest failed  Total time for all tests: 0.3 seconds cpu time: 0.3 seconds cumulative wall time: 0.3 seconds
comment:15 Changed 4 years ago by
should the following be applied:
 a/src/sage/combinat/designs/design_catalog.py +++ b/src/sage/combinat/designs/design_catalog.py @@ 74,7 +74,7 @@ REFERENCES: TESTS::  sage: 'absolute_import' in dir(designs) or 'deprecated_callable_import' in dir(designs) + sage: 'deprecated_callable_import' in dir(designs) False """ from __future__ import absolute_import
(I don't get the absolute_import
part in this testsurely absolute_import
is all over the place; removing it from the test makes the test pass)
comment:16 Changed 4 years ago by
 Commit changed from 18e9a467945579b5fc907de02a46bef4eb888f0f to e258a2053f99656bc3ee079f796758039d994750
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
179ac5e  cleanup of Griesmer bound, docs and examples

bb33393  Merge branch 'u/dimpase/codesizefix' of trac.sagemath.org:sage into codesizefix

224b1be  Remove rename_keyword deprecation warning (6 years old)

1ee9d38  Basic parameter sanity check on more bounds

4b0a57e  More docstring fixes

5d2ee22  Merge branch 'u/jsrn/codesizefix' of trac.sagemath.org:sage into codesizefix

e793a92  added field_based and used it whenever needed

228451d  adjusted the top docs to talk about codes not only over fields

e398f4c  Merge branch 'u/dimpase/codesizefix' of trac.sagemath.org:sage into devbeta6

e258a20  remove obsolete test

comment:17 Changed 4 years ago by
oops, wrong branch
comment:18 Changed 4 years ago by
 Commit changed from e258a2053f99656bc3ee079f796758039d994750 to f92d174b5e9890f57b14aeb9552cdb1399b268d4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f92d174  remove obsolete test

comment:19 followup: ↓ 20 Changed 4 years ago by
 Status changed from needs_work to needs_review
the last commit removes an obsolete test  there is no deprecated_callable_import
anywhere any more anyway.
comment:20 in reply to: ↑ 19 Changed 4 years ago by
Replying to dimpase:
the last commit removes an obsolete test  there is no
deprecated_callable_import
anywhere any more anyway.
Well, the test was about absolute_import
and deprecated_callable_import
. You only fixed the latter, I now fixed the former too.
comment:21 Changed 4 years ago by
 Branch changed from u/dimpase/t22796 to u/jdemeyer/t22796
comment:22 Changed 4 years ago by
 Commit changed from f92d174b5e9890f57b14aeb9552cdb1399b268d4 to 0ac7893146826afa62ffe437868e8c5d6d313c17
Sage imports drive me mad elsewhere too: https://trac.sagemath.org/ticket/22799#comment:90 :)
New commits:
0ac7893  Remove absolute_import from catalog

comment:23 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:24 Changed 4 years ago by
 Branch changed from u/jdemeyer/t22796 to 0ac7893146826afa62ffe437868e8c5d6d313c17
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Remove deprecated_callable_import