[issue11572] bring Lib/copy.py to 100% coverage

Éric Araujo added the comment: I realize now that calling self.fail at https://hg.python.org/cpython/rev/74e79b2c114a#l2.20 is a problem: self is an instance of the C class, not the TestCase instance. (The line is unreachable anyway so this doesn’t matter a lot. In other projects I’d use

[issue11572] bring Lib/copy.py to 100% coverage

Berker Peksag added the comment: Thanks, Brandon. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed versions: +Python 3.8 -Python 3.7 ___ Python tracker

[issue11572] bring Lib/copy.py to 100% coverage

Berker Peksag added the comment: New changeset 2708578736d1aa15685495e9b94b827a8e185a8c by Berker Peksag in branch 'master': bpo-11572: Make minor improvements to copy module (GH-8208) https://github.com/python/cpython/commit/2708578736d1aa15685495e9b94b827a8e185a8c --

[issue11572] bring Lib/copy.py to 100% coverage

Change by Berker Peksag : -- pull_requests: +7757 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue11572] bring Lib/copy.py to 100% coverage

Serhiy Storchaka added the comment: copy and pickle use the same protocol. The copy module shouldn't be changed independently, the changes should be synchronised with both implementations of the pickle module. -- ___ Python tracker

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Berker Peksag : -- stage: needs patch -> patch review ___ Python tracker ___

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Berker Peksag : -- pull_requests: +176 ___ Python tracker ___ ___

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Serhiy Storchaka : -- stage: patch review -> needs patch versions: +Python 3.7 -Python 3.5 ___ Python tracker ___

[issue11572] bring Lib/copy.py to 100% coverage

Serhiy Storchaka added the comment: Agree. The latter simplification can be applied to pickle.py. -- ___ Python tracker ___

[issue11572] bring Lib/copy.py to 100% coverage

Berker Peksag added the comment: All changes to Lib/test/test_copy.py have already been committed. Perhaps the following hunks from the latest patch can still be useful: -try: -d[types.CodeType] = _deepcopy_atomic -except AttributeError: -pass +d[types.CodeType] = _deepcopy_atomic ---

[issue11572] bring Lib/copy.py to 100% coverage

Serhiy Storchaka added the comment: Is anything left to do with this issue? Many changes were made in the copy module and tests last year. -- nosy: +serhiy.storchaka status: open -> pending ___ Python tracker

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Berker Peksag berker.pek...@gmail.com: -- components: +Library (Lib) nosy: +berker.peksag type: - enhancement versions: +Python 3.5 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11572

[issue11572] bring Lib/copy.py to 100% coverage

Sandro Tosi sandro.t...@gmail.com added the comment: JFTR, the same kind of check of __reduce_ex__ and then falling back on __reduce__ is prenset in pickle too: maybe it's worth align them? -- ___ Python tracker rep...@bugs.python.org

[issue11572] bring Lib/copy.py to 100% coverage

Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 74e79b2c114a by Sandro Tosi in branch 'default': #11572: improvements to copy module tests along with removal of old test suite http://hg.python.org/cpython/rev/74e79b2c114a -- nosy: +python-dev

[issue11572] bring Lib/copy.py to 100% coverage

Sandro Tosi sandro.t...@gmail.com added the comment: Brandon, thanks for your work on this patch! I've just committed the unittests update+removal of _test() part. For the remaining part, I see that Nick and you worked on it during a sprint, so I'm quite sure it's fine, but nonetheless it

[issue11572] bring Lib/copy.py to 100% coverage

Sandro Tosi sandro.t...@gmail.com added the comment: Hi Brandon, I really like to see your patch applied, let's see what I can do (I also added Ezio in the loop). I think you only addressed half of msg132140 : could you please have a look at the first Éric's question? Also, still Éric made a

[issue11572] bring Lib/copy.py to 100% coverage

Ezio Melotti ezio.melo...@gmail.com added the comment: ISTM that the patch is trying to do too many things at once: 1) increase the test coverage, possibly fixing some bugs discovered while doing so; 2) refactor the tests to use the correct assert methods; 3) get rid of old code, and do some

[issue11572] bring Lib/copy.py to 100% coverage

Sandro Tosi sandro.t...@gmail.com added the comment: After a quick chat with Ezio, we tried to revert the changes to copy.py while keeping the ones on test, and the test suite passes. The next steps would probably be to just commit the diff for test_copy.py and see if the changes on copy.py

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Ezio and Sandro, thank you very much for your attention to this issue, and for helping me split it into manageable chunks! To answer the question about why coverage does not show as high a total as it ought: it's because coverage

[issue11572] bring Lib/copy.py to 100% coverage

Nick Coghlan ncogh...@gmail.com added the comment: I'd assigned this to myself since I was discussing it with Brandon when he was working on it at the PyCon sprints. Since I certainly don't want to block anyone else getting to it, I'm unassigning it - feel free to take it forward :) IIRC, the

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Éric, I think your points are good ones. (And, as I return to this patch after three months, I should thank the PSF for sponsoring the CPython sprint here at PyOhio, and creating this opportunity for me to continue trying to land

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Brett Cannon br...@python.org: -- keywords: +needs review stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11572 ___

[issue11572] bring Lib/copy.py to 100% coverage

Éric Araujo mer...@netwok.org added the comment: Thanks for the updated patch. -t = getattr(types, CodeType, None) -if t is not None: -d[t] = _copy_immutable +d[types.CodeType] = _copy_immutable What was the use case for this again? The defunct restricted mode, another VM or something

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Nick Coghlan rep...@bugs.python.org writes: Regarding __reduce__, other readers will have the same question Éric did, so that point should definitely go in a comment after the __reduce_ex__ check. I just sat down to review this

[issue11572] bring Lib/copy.py to 100% coverage

Éric Araujo mer...@netwok.org added the comment: The suggestion about assertIsInstance was a mistake, I misread issubclass in the original code. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11572

[issue11572] bring Lib/copy.py to 100% coverage

Éric Araujo mer...@netwok.org added the comment: Nice cleanup. -reductor = getattr(x, __reduce__, None) -if reductor: -rv = reductor() -else: -raise Error(un(shallow)copyable object of type %s % cls) +raise

[issue11572] bring Lib/copy.py to 100% coverage

Nick Coghlan ncogh...@gmail.com added the comment: Regarding __reduce__, other readers will have the same question Éric did, so that point should definitely go in a comment after the __reduce_ex__ check. -- ___ Python tracker rep...@bugs.python.org

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Nick Coghlan rep...@bugs.python.org writes: Nick Coghlan ncogh...@gmail.com added the comment: Regarding __reduce__, other readers will have the same question Éric did, so that point should definitely go in a comment after the

[issue11572] bring Lib/copy.py to 100% coverage

Éric Araujo mer...@netwok.org added the comment: A little research has found that building without complex is not possible anymore, so you’re good: http://bugs.python.org/issue7147 Regarding “unicode”, see line 112. -- ___ Python tracker

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Éric, after checking line 112 of the two patches and then of the new file, I figured out that you meant line 112 of the old file — and, yes, that test can go away too since in python3 complex always exists and unicode never

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Daniel Urban urban.dani...@gmail.com: -- nosy: +durban ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11572 ___ ___ Python-bugs-list

[issue11572] bring Lib/copy.py to 100% coverage

Éric Araujo mer...@netwok.org added the comment: Some code is removed by your patch, for example a check that prevented an error for builds that don’t include complex. If this type of builds still exist, the code should be compatible. Conversely, the code mentions unicode in some places, but

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Éric, the Makefile in Python trunk seems to include Objects/complexobject.o in the build unilaterally without any way to turn it off. What is leading you to believe that Python 3 can conditionally turn the complex type off during a

[issue11572] bring Lib/copy.py to 100% coverage

Antoine Pitrou pit...@free.fr added the comment: +1 for not having pragma statements in the stdlib, especially when they target a third-party tool few of us know and use. Beside, while coverage measurements are useful, trusting them that actual coverage is complete is IMO a bit foolish. It's

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Antoine, neither this issue, nor either version of my patch, was intended to assert that 100% test coverage indicates that a test of tests are complete. If you will point out where in the text this is implied, I will correct it.

[issue11572] bring Lib/copy.py to 100% coverage

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: On Thu, Mar 17, 2011 at 8:03 PM, Antoine Pitrou rep...@bugs.python.org wrote: .. +1 for not having pragma statements in the stdlib, especially when they target a third-party tool few of us know and use. #pragma NO COVER

[issue11572] bring Lib/copy.py to 100% coverage

New submission from Brandon Craig Rhodes bran...@rhodesmill.org: The attached patch will bring Lib/copy.py to 100% test coverage. A bug in coverage results in its only reporting 99% at the moment; see coverage issue #122 on bitbucket:

[issue11572] bring Lib/copy.py to 100% coverage

Benjamin Peterson benja...@python.org added the comment: 1. I prefer that we don't have pragma statements sprinkled over the stdlib. 2. You can use assertIs() instead of assertTrue(x is y) (Feel free to change the lot) 3. Along the way you could also change those raise support.TestFailed over

[issue11572] bring Lib/copy.py to 100% coverage

Alexander Belopolsky belopol...@users.sourceforge.net added the comment: A bug in coverage results in its only reporting 99% at the moment It was concluded during discussion of issue2506 that this is not a bug. At least not a bug in coverage or the trace module. At most this is a peephole

[issue11572] bring Lib/copy.py to 100% coverage

Changes by Nick Coghlan ncogh...@gmail.com: -- assignee: - ncoghlan nosy: +ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11572 ___ ___

[issue11572] bring Lib/copy.py to 100% coverage

Nick Coghlan ncogh...@gmail.com added the comment: Alexander: the coverage problem in this case has to do with it incorrectly handling an else: clause on a loop (it doesn't adjust the expected target for an empty sequence to be the body of the else clause) Benjamin: the pragma question is

[issue11572] bring Lib/copy.py to 100% coverage

Benjamin Peterson benja...@python.org added the comment: 2011/3/16 Nick Coghlan rep...@bugs.python.org: Nick Coghlan ncogh...@gmail.com added the comment: Alexander: the coverage problem in this case has to do with it incorrectly handling an else: clause on a loop (it doesn't adjust the

[issue11572] bring Lib/copy.py to 100% coverage

Brandon Craig Rhodes bran...@rhodesmill.org added the comment: Benjamin, thanks for the pointers! The attached patch now uses assertIs() and assertIsNot(), and calls self.fail() instead of using the exception from support. In the future I would like some way to determine when test coverage is