#6521 closed enhancement (fixed)
[with patch, positive review] replace .copy() with .__copy__() in matrix/matrix0.pyx
Reported by: | AlexGhitza | Owned by: | was |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.1.2 |
Component: | user interface | Keywords: | |
Cc: | Merged in: | Sage 4.1.2.alpha2 | |
Authors: | Karl-Dieter Crisman, Jason Grout | Reviewers: | Jason Grout, Karl-Dieter Crisman, Minh Van Nguyen |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
See also #111, where this originates.
Attachments (3)
Change History (16)
Changed 12 years ago by
comment:1 Changed 12 years ago by
- Summary changed from replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, needs review] replace .copy() with .__copy__() in matrix/matrix0.pyx
This passed all doctests in sage.matrix, sage.modules, and sage.groups.matrix_gps, so hopefully I got all of them.
comment:2 follow-up: ↓ 3 Changed 12 years ago by
Is there a deprecation warning now on .copy()? I didn't see one on this patch.
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 12 years ago by
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 12 years ago by
Replying to kcrisman:
Replying to jason:
Is there a deprecation warning now on .copy()? I didn't see one on this patch.
Does there need to be? I don't recall that in #111 there was, but maybe there is supposed to be?
Yes, there does need to be a deprecation warning. #111 was *way* before we had the deprecation warning policy, so that's why it wasn't mentioned there. Unfortunately, I think a lot of code (including my code!) has used the .copy() function since then that would break. We need to give us poor folks a warning that things are changing.
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 12 years ago by
- Summary changed from [with patch, needs review] replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, needs work] replace .copy() with .__copy__() in matrix/matrix0.pyx
Replying to jason:
Replying to kcrisman:
Replying to jason:
Is there a deprecation warning now on .copy()? I didn't see one on this patch.
Does there need to be? I don't recall that in #111 there was, but maybe there is supposed to be?
Yes, there does need to be a deprecation warning. #111 was *way* before we had the deprecation warning policy, so that's why it wasn't mentioned there. Unfortunately, I think a lot of code (including my code!) has used the .copy() function since then that would break. We need to give us poor folks a warning that things are changing.
Unfortunately, the patch for #111 was only merged a few weeks ago, and is new in 4.1.1! That means that you should probably open a followup patch for putting deprecation warnings in for all of those. And then opening another patch that says to remove them in 6 months...
comment:6 in reply to: ↑ 5 Changed 12 years ago by
Replying to kcrisman:
Unfortunately, the patch for #111 was only merged a few weeks ago, and is new in 4.1.1! That means that you should probably open a followup patch for putting deprecation warnings in for all of those. And then opening another patch that says to remove them in 6 months...
Ouch, okay. I'll post a quick patch to this ticket (since I know at least some of my stuff will break) and open up a follow-up ticket for the rest of the stuff in #111.
comment:7 Changed 12 years ago by
- Summary changed from [with patch, needs work] replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, needs review] replace .copy() with .__copy__() in matrix/matrix0.pyx
positive review for kcrisman's patch.
kcrisman, can you review my patch? It adds the deprecation warning.
comment:8 Changed 12 years ago by
- Reviewers set to Jason Grout
comment:9 Changed 12 years ago by
- Reviewers changed from Jason Grout to Jason Grout, Karl-Dieter Crisman
- Summary changed from [with patch, needs review] replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, positive review] replace .copy() with .__copy__() in matrix/matrix0.pyx
Looks okay, passes relevant tests. I hadn't seen that doctest:...: thing before, it's good to know about.
comment:10 Changed 12 years ago by
- Summary changed from [with patch, positive review] replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, needs work] replace .copy() with .__copy__() in matrix/matrix0.pyx
I got the following doctest failures. Except for the twist.py related failure, these are all a direct consequence of having deprecated the method .copy()
.
sage -t -long devel/sage/sage/server/simple/twist.py ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/server/simple/twist.py", line 51: sage: print get_url('http://localhost:%s/simple/compute?session=%s&code=2*2' % (port, session)) Expected: { "status": "done", "files": [], "cell_id": 1 } ___S_A_G_E___ 4 Got: { "status": "done", "files": [], "cell_id": 1 } ___S_A_G_E___ <BLANKLINE> 4 e0 ********************************************************************** 1 items had failures: 1 of 24 in __main__.example_0 ***Test Failed*** 1 failures. For whitespace errors, see the file /scratch/mvngu/release/sage-4.1.2.alpha1/tmp/.doctest_twist.py [8.5 s] <SNIP> sage -t -long devel/sage/sage/geometry/lattice_polytope.py ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/geometry/lattice_polytope.py", line 233: sage: p = LatticePolytope(m, "A lattice polytope with WRONG vertices", compute_vertices=False) Expected nothing Got: doctest:259: DeprecationWarning: the .copy() method is deprecated; please use the copy() function instead, for example, copy(M) ********************************************************************** 1 items had failures: 1 of 18 in __main__.example_3 ***Test Failed*** 1 failures. For whitespace errors, see the file /scratch/mvngu/release/sage-4.1.2.alpha1/tmp/.doctest_lattice_polytope.py [11.9 s] <SNIP> sage -t -long devel/sage/sage/plot/plot3d/base.pyx ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/plot/plot3d/base.pyx", line 1509: sage: T.get_matrix() Expected: [100.0 0.0 0.0 0.0] [ 0.0 100.0 0.0 0.0] [ 0.0 0.0 100.0 0.0] [ 0.0 0.0 0.0 1.0] Got: doctest:1172: DeprecationWarning: the .copy() method is deprecated; please use the copy() function instead, for example, copy(M) [100.0 0.0 0.0 0.0] [ 0.0 100.0 0.0 0.0] [ 0.0 0.0 100.0 0.0] [ 0.0 0.0 0.0 1.0] ********************************************************************** 1 items had failures: 1 of 5 in __main__.example_55 ***Test Failed*** 1 failures. For whitespace errors, see the file /scratch/mvngu/release/sage-4.1.2.alpha1/tmp/.doctest_base.py [8.0 s] <SNIP> sage -t -long devel/sage/sage/crypto/mq/sbox.py ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/crypto/mq/sbox.py", line 420: sage: S.maximal_difference_probability_absolute() Expected: 2 Got: doctest:1: DeprecationWarning: the .copy() method is deprecated; please use the copy() function instead, for example, copy(M) 2 ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/crypto/mq/sbox.py", line 440: sage: S.maximal_difference_probability() Expected: 0.25 Got: doctest:443: DeprecationWarning: the .copy() method is deprecated; please use the copy() function instead, for example, copy(M) 0.25 ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/crypto/mq/sbox.py", line 518: sage: S.maximal_linear_bias_absolute() Expected: 2 Got: doctest:1: DeprecationWarning: the .copy() method is deprecated; please use the copy() function instead, for example, copy(M) 2 ********************************************************************** File "/scratch/mvngu/release/sage-4.1.2.alpha1/devel/sage-main/sage/crypto/mq/sbox.py", line 533: sage: S.maximal_linear_bias_relative() Expected: 0.25 Got: doctest:536: DeprecationWarning: the .copy() method is deprecated; please use the copy() function instead, for example, copy(M) 0.25 ********************************************************************** 4 items had failures: 1 of 4 in __main__.example_14 1 of 4 in __main__.example_15 1 of 4 in __main__.example_17 1 of 4 in __main__.example_18 ***Test Failed*** 4 failures. For whitespace errors, see the file /scratch/mvngu/release/sage-4.1.2.alpha1/tmp/.doctest_sbox.py [3.0 s]
comment:11 Changed 12 years ago by
- Summary changed from [with patch, needs work] replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, needs review] replace .copy() with .__copy__() in matrix/matrix0.pyx
The -fixes.patch should take care of all of those doctest failures.
comment:12 Changed 12 years ago by
- Merged in set to Sage 4.1.2.alpha2
- Resolution set to fixed
- Reviewers changed from Jason Grout, Karl-Dieter Crisman to Jason Grout, Karl-Dieter Crisman, Minh Van Nguyen
- Status changed from new to closed
Merged patches in this order:
trac_6521-copy-matrix.patch
trac_6521-deprecation.patch
trac_6521-deprecation-fixes.patch
All doctests pass.
comment:13 Changed 12 years ago by
- Summary changed from [with patch, needs review] replace .copy() with .__copy__() in matrix/matrix0.pyx to [with patch, positive review] replace .copy() with .__copy__() in matrix/matrix0.pyx
Based on 4.1.1