Opened 7 years ago
Closed 6 years ago
#14329 closed enhancement (fixed)
Remove all the "is_[AZ].*" from global namespace
Reported by:  ppurka  Owned by:  tbd 

Priority:  minor  Milestone:  sage6.3 
Component:  performance  Keywords:  
Cc:  Merged in:  
Authors:  Marc Mezzarobba  Reviewers:  Peter Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  b948874 (Commits)  Commit:  b9488746a27bb73eee93ad81d1bea1d0ddfec2f5 
Dependencies:  #15076, #15098, #15333  Stopgaps: 
Description (last modified by )
Remove all the is_[AZ].*
from global namespace. They have been deprecated for over two years.
step 1) remove the imports of is_* from rings/all.py (#15076)
step 2) remove the imports of is_* from matrix/all.py (#15098)
step 3) idem for modules/all.py and structure/all.py (#15333)
step 4) remove otherwise unused (i.e., not reimported) imports of is_[AZ]* (this ticket)
step 5) remove the remaining imports of is_[AZ]* (this ticket)
Change History (32)
comment:1 Changed 7 years ago by
comment:2 Changed 7 years ago by
 Description modified (diff)
 Summary changed from Remove all the `is_[AZ]*` from global namespace to Remove all the "is_[AZ].*" from global namespace
Arrgghhh! regexp!! There, the current modification should suffice.
On a serious note, what I do mean is all the "is_Vector", "is_Polynomial", "is_Matrix", and what not. The patch will be trivial but tiresome. I don't know any means of listing and testing whether they give a deprecation warning automatically. So, it has to be a slow, manual, and visual process.
comment:3 Changed 7 years ago by
import re from subprocess import check_output, STDOUT, CalledProcessError for x in globals().keys(): if re.match("is_[AZ]", x) is not None: try: output = check_output(['sage', 'c', x + '(None)'], stderr=STDOUT) except CalledProcessError as e: output = e.output if re.match(".*DeprecationWarning", output) is not None: print x
is a hack but should work.
comment:4 Changed 7 years ago by
(cc me)
comment:5 Changed 7 years ago by
 Milestone changed from sage5.9 to sageduplicate/invalid/wontfix
 Status changed from new to needs_review
This is a duplicate of #12824.
comment:6 Changed 7 years ago by
I was definitely not advocating removing the functions themselves. They can still be used internally. Is it desired that the functions themselves are removed?
comment:7 Changed 7 years ago by
Since they've already been deprecated (from the global namespace in #10107), the trivial ones can be removed since we don't really want to use them IMO (a extra unnecessary python function call). Perhaps we could just use this ticket to explicitly remove these functions only from the global namespace (as a step towards #12824)?
comment:8 Changed 7 years ago by
Yes. That was my intention. :)
comment:9 Changed 7 years ago by
 Milestone changed from sageduplicate/invalid/wontfix to sage5.9
 Status changed from needs_review to needs_work
Ah okay, then this patch would just be a matter of finding which all.py
files are importing these...
comment:10 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:11 Changed 6 years ago by
As a first step I propose #15076
comment:12 Changed 6 years ago by
 Description modified (diff)
comment:13 Changed 6 years ago by
 Description modified (diff)
step 1) should be ok and is wating for review at #15076
comment:14 Changed 6 years ago by
 Description modified (diff)
comment:15 Changed 6 years ago by
 Description modified (diff)
comment:16 Changed 6 years ago by
 Description modified (diff)
comment:17 Changed 6 years ago by
 Description modified (diff)
comment:18 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:19 Changed 6 years ago by
 Branch set to u/mmezzarobba/14329remove_is_foo
 Commit set to 04a9dc703fec1f6facd9837afc6aff6f53335112
 Dependencies set to #15076, #15098, #15333
 Priority changed from major to minor
 Status changed from needs_work to needs_review
comment:20 Changed 6 years ago by
comment:21 followup: ↓ 22 Changed 6 years ago by
For a ticket like that, it is very important to test sage in full. Did you do that ?
comment:22 in reply to: ↑ 21 Changed 6 years ago by
Replying to chapoton:
For a ticket like that, it is very important to test sage in full. Did you do that ?
I ran make ptest
, but not # long time
nor optional tests (...but the few doctests matching sage:.*is_.*optional
seem to properly import the predicate they call).
comment:23 Changed 6 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:24 followup: ↓ 25 Changed 6 years ago by
 Status changed from needs_review to needs_info
There are still dozens of is_*
functions in the global namespace; is_Radix64StringMonoidElement
, just to name a random example arising from typing is_[tab]
in Sage 6.3.beta1. Shouldn't there be some more steps like 13 in the ticket description before we can remove the warning?
comment:25 in reply to: ↑ 24 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_info to needs_work
Replying to pbruin:
There are still dozens of
is_*
functions in the global namespace;is_Radix64StringMonoidElement
, just to name a random example arising from typingis_[tab]
in Sage 6.3.beta1. Shouldn't there be some more steps like 13 in the ticket description before we can remove the warning?
Looks like you are completely right! I don't remember why I thought we were done.
Btw many of the imports that were supposed to be removed as part of steps 1)3) are still there (or were added back?).
comment:26 Changed 6 years ago by
 Commit changed from 04a9dc703fec1f6facd9837afc6aff6f53335112 to 064e29f3c722a0a56e893ec760a5c79e261e6a1b
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
ef52d40  rm global is_EllipticCurve

275ecb3  rm global is_HyperellipticCurve

e055984  rm global is_{Expect,Gap,Singular}Element

b0d2cd0  rm global is_Finite_Field, is_Finite_FieldElement

9d2363d  rm global is_Graphics

03fb568  rm global is_ProjectiveSpace

07f24db  rm global is_QuadraticForm

ccb3ac7  rm global is_Scheme

da22e56  rm global is_Morphism

064e29f  global is_*: delete deprecation warning

comment:27 Changed 6 years ago by
 Commit changed from 064e29f3c722a0a56e893ec760a5c79e261e6a1b to a616a717e1c2f4a2790b864856ccb91bdeca02d6
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
9e0f8ad  rm global is_EllipticCurve

dd3e424  rm global is_HyperellipticCurve

e71d0cb  rm global is_{Expect,Gap,Singular}Element

0aa3b40  rm global is_Finite_Field, is_Finite_FieldElement

b0dc1cf  rm global is_Graphics

de6835b  rm global is_ProjectiveSpace

037eae0  rm global is_QuadraticForm

5af32ad  rm global is_Scheme

95ddc08  rm global is_Morphism

a616a71  global is_*: delete deprecation warning

comment:28 Changed 6 years ago by
Work in progress, only partially tested, expect forced pushes!
comment:29 Changed 6 years ago by
 Commit changed from a616a717e1c2f4a2790b864856ccb91bdeca02d6 to b9488746a27bb73eee93ad81d1bea1d0ddfec2f5
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
638875b  rm global is_EllipticCurve

e620d38  rm global is_HyperellipticCurve

1932b64  rm global is_{Expect,Gap,Singular}Element

3841cc7  rm global is_Finite_Field, is_Finite_FieldElement

ae72be9  rm global is_Graphics

f0ac47d  rm global is_ProjectiveSpace

bd2b01a  rm global is_QuadraticForm

fa34d62  rm global is_Scheme

8ad8402  rm global is_Morphism

b948874  global is_*: delete deprecation warning

comment:30 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
This time there are no is_[AZ]*
remaining in the global namespace and all (long) tests pass.
Note that I did not remove is_pAdic{Ring,Field}
since these were not formally deprecated as far as I understand (but I would not mind removing them as well if other people agree).
comment:31 Changed 6 years ago by
 Reviewers set to Peter Bruin
 Status changed from needs_review to positive_review
I would be in favour of removing is_pAdic{Ring,Field}
as well, but it's better to deprecate them first. We can do it in a future round of cleaning up the global namespace.
comment:32 Changed 6 years ago by
 Branch changed from u/mmezzarobba/14329remove_is_foo to b9488746a27bb73eee93ad81d1bea1d0ddfec2f5
 Resolution set to fixed
 Status changed from positive_review to closed
Do you mean
is_[AZ][AZaz]*
?