Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#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:

Status badges

Description

See also #111, where this originates.

Attachments (3)

trac_6521-copy-matrix.patch (31.1 KB) - added by kcrisman 12 years ago.
Based on 4.1.1
trac_6521-deprecation.patch (2.4 KB) - added by jason 12 years ago.
apply on top of previous patch
trac_6521-deprecation-fixes.patch (2.0 KB) - added by jason 12 years ago.
apply on top of previous patches

Download all attachments as: .zip

Change History (16)

Changed 12 years ago by kcrisman

Based on 4.1.1

comment:1 Changed 12 years ago by kcrisman

  • 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: Changed 12 years ago by jason

Is there a deprecation warning now on .copy()? I didn't see one on this patch.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 12 years ago by 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?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 12 years ago by 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.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 12 years ago by kcrisman

  • 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 jason

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.

Changed 12 years ago by jason

apply on top of previous patch

comment:7 Changed 12 years ago by jason

  • 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 jason

  • Authors set to Karl-Dieter Crisman, Jason Grout
  • Reviewers set to Jason Grout

comment:9 Changed 12 years ago by kcrisman

  • 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 mvngu

  • 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]

Changed 12 years ago by jason

apply on top of previous patches

comment:11 Changed 12 years ago by jason

  • 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 mvngu

  • 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:

  1. trac_6521-copy-matrix.patch
  2. trac_6521-deprecation.patch
  3. trac_6521-deprecation-fixes.patch

All doctests pass.

comment:13 Changed 12 years ago by mvngu

  • 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
Note: See TracTickets for help on using tickets.