Drop py27 and py34 support #5318

nicoddemus · 2019-05-27T23:41:56Z

Related to #5275

codecov · 2019-05-27T23:51:35Z

Codecov Report

Merging #5318 into master will increase coverage by 0.99%.
The diff coverage is 98.44%.

@@            Coverage Diff             @@
##           master    #5318      +/-   ##
==========================================
+ Coverage   93.43%   94.43%   +0.99%     
==========================================
  Files         115      115              
  Lines       26399    25895     -504     
  Branches     2606     2496     -110     
==========================================
- Hits        24667    24453     -214     
+ Misses       1409     1126     -283     
+ Partials      323      316       -7

Impacted Files	Coverage Δ
testing/test_warnings.py	`61.21% <ø> (+12.03%)`	⬆️
src/_pytest/assertion/__init__.py	`89.55% <ø> (+2.59%)`	⬆️
src/_pytest/doctest.py	`95.5% <ø> (-0.93%)`	⬇️
src/_pytest/recwarn.py	`98.09% <ø> (-0.3%)`	⬇️
testing/python/metafunc.py	`93.55% <ø> (+6.75%)`	⬆️
testing/test_compat.py	`100% <ø> (ø)`	⬆️
src/_pytest/warnings.py	`87.32% <ø> (-1.57%)`	⬇️
testing/test_config.py	`99.82% <ø> (ø)`	⬆️
src/_pytest/junitxml.py	`95.88% <ø> (-0.03%)`	⬇️
testing/test_tmpdir.py	`98.36% <ø> (-0.57%)`	⬇️
... and 76 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 733f43b...4d49ba6. Read the comment docs.

nicoddemus · 2019-05-28T00:04:09Z

Oh coverage will drop quite a bit because of dead Python 2 code (which we should tackle in future PRs).

tox.ini

blueyed · 2019-05-28T00:08:04Z

which we should tackle in future PRs

Why not here?
We have the coverage indicator here, but not later anymore.

nicoddemus · 2019-05-28T00:12:18Z

Why not here?
We have the coverage indicator here, but not later anymore.

Fair enough. I will get to this tomorrow or after 4.6 gets out. 👍

setup.cfg

setup.py

testing/python/integration.py

setup.cfg

blueyed · 2019-05-28T16:36:16Z

Pushed a minor commit removing one instance/import of six - it should get removed completely (added a TODO).

asottile · 2019-05-28T16:38:51Z

pyupgrade will handle that @blueyed -- no need to make those changes here

blueyed · 2019-05-28T16:47:15Z

@asottile
Ah, sure - should be run here then already, no?

asottile · 2019-05-28T16:48:34Z

it was my understanding from #5275 that we were going to do that in two separate branches (this one, and then I'd follow up with a separate pyupgrade branch)

blueyed · 2019-05-28T17:21:23Z

It would help / is required for the coverage - otherwise you would have to compare this "manually" from before this PR and after the pyupgrade one.
I think it is good to have this all in one place.

asottile · 2019-05-28T17:22:59Z

🤷‍♂️ I don't think the coverage number matters too much -- I'd prefer two separate PRs personally

nicoddemus · 2019-05-28T18:44:01Z

About the two separate PRs:

One problem with just pushing manual + automatic changes in the same PR is that it makes it difficult to review the manual changes. I'm sure if we had just pushed the changes we have here in addition to automatic changes introduced by pyupgrade, the comments made here might not have been made at all among all the noise. On the other hand, I understand @blueyed's sentiment about having a clear place to check for the absolute coverage.

Having said that, how about:

We keep the scope of the PR as it is now, only the manual changes I've made + removing the if six.P* blocks. I plan to finish this today.
Once this is done, people should request further changes and finally approve it.
Once everyone has approved this, instead of merging it, @asottile then pushes the automatic changes to this PR.
Everybody now has a chance of requesting further changes until we finally merge it (after 4.6 is out of course).

This might seem complicated at first sight, but is in essence just "approve it now what you see, later we will push more changes before merging".

What do you guys think?

asottile · 2019-05-28T18:50:51Z

We keep the scope of the PR as it is now, only the manual changes I've made + removing the if six.P* blocks. I plan to finish this today.

pyupgrade is about to support this as well, mostly done :) asottile/pyupgrade#153

nicoddemus · 2019-05-28T19:10:05Z

pyupgrade is about to support this as well, mostly done :) asottile/pyupgrade#153

Cool!

I will do it myself later anyway, a few of places are not straightforward to remove cleanly.

Some examples:

pytest/src/_pytest/pathlib.py

Lines 87 to 95 in 81cc731

    
           if six.PY2: 
        
               def _max(iterable, default): 
        
                   """needed due to python2.7 lacking the default argument for max""" 
        
                   return reduce(max, iterable, default) 
        
           else: 
        
               _max = max

Here it makes sense to use max directly instead of _max, assuming pyupgrade will keep just _max = max.

pytest/src/_pytest/capture.py

Lines 842 to 850 in 81cc731

    
           def _attempt_to_close_capture_file(f): 
        
               """Suppress IOError when closing the temporary file used for capturing streams in py27 (#2370)""" 
        
               if six.PY2: 
        
                   try: 
        
                       f.close() 
        
                   except IOError: 
        
                       pass 
        
               else: 
        
                   f.close()

Here it makes sense to also remove _attempt_to_close_capture_file and call f.close() directly instead.

Also there are tests which are Python 2 only which should be removed completely.

There other exmaples, although most would be handled by pyupgrade just fine. 😁

nicoddemus · 2019-05-29T00:37:21Z

OK, removed everything I could find that was related to Python 2 in the code: code blocks with sys.version_info, six.PY2, six.PY3, _pytest.compat functions no longer necessary, etc. 😅

asottile

looks good, feel free to ignore the issue -- we can always follow up with another round of cleanup afterwards :)

I'm going to wait to pyupgrade this until we release 4.6 so we can rebase on top of that

asottile · 2019-05-29T04:53:32Z

src/_pytest/_code/code.py

+            else:
+                e = None
+        repr_chain.reverse()
+        return ExceptionChainRepr(repr_chain)


 class TerminalRepr(object):
    def __str__(self):
        s = self.__unicode__()


v minor, we should be able to get rid of __unicode__ for __str__

nicoddemus · 2019-05-29T11:22:27Z

v minor, we should be able to get rid of unicode for str

Good catch, missed it. I will get to this later.

nicoddemus · 2019-05-29T11:24:10Z

I'm going to wait to pyupgrade this until we release 4.6 so we can rebase on top of that

Yes, I was going to suggest that. We should do that right before merging this PR, after 4.6 is out. 👍

src/_pytest/compat.py

...

.travis.yml

auvipy

great work. just need re-base I guess

nicoddemus · 2019-06-02T14:50:25Z

Rebased/squased into master: we have already created the 4.6-maintenance branch, so this can go directly to master.

Also I think we can run pyupgrade in a separate PR, as it should not affect coverage at all.

asottile

so happy to see all these hacks disappear!

doc/en/example/markers.rst

asottile · 2019-06-02T15:15:54Z

sweet, let's get this into master :D

RonnyPfannschmidt · 2019-06-02T16:02:07Z

src/_pytest/compat.py

-
-if _PY3:
-
-    def safe_str(v):


i noted this one is evil, looking at usage sites made me think the removal was bad in some case

What do you mean @RonnyPfannschmidt? The Python 3 version was just calling str(), so I changed the call sites to call str() as well and removed the safe_str function.

@nicoddemus the function name was terrible for indicating the actual intent

Oh OK.

Could you please "accept" the PR once you finished the review? It would be nice to get this into master soon.

* Update setup.py requires and classifiers * Drop Python 2.7 and 3.4 from CI * Update docs dropping 2.7 and 3.4 support * Fix mock imports and remove tests related to pypi's mock module * Add py27 and 34 support docs to the sidebar * Remove usage of six from tmpdir * Remove six.PY* code blocks * Remove sys.version_info related code * Cleanup compat * Remove obsolete safe_str * Remove obsolete __unicode__ methods * Remove compat.PY35 and compat.PY36: not really needed anymore * Remove unused UNICODE_TYPES * Remove Jython specific code * Remove some Python 2 references from docs Related to pytest-dev#5275

nicoddemus · 2019-06-02T22:15:00Z

🎉

nicoddemus changed the title ~~[WIP] Drop py27 py34~~ [WIP] Drop py27 and py34 support May 27, 2019

blueyed reviewed May 28, 2019

View reviewed changes

tox.ini Show resolved Hide resolved

blueyed previously approved these changes May 28, 2019

View reviewed changes

asottile reviewed May 28, 2019

View reviewed changes

setup.cfg Show resolved Hide resolved

RonnyPfannschmidt reviewed May 28, 2019

View reviewed changes

setup.py Show resolved Hide resolved

RonnyPfannschmidt reviewed May 28, 2019

View reviewed changes

testing/python/integration.py Outdated Show resolved Hide resolved

RonnyPfannschmidt reviewed May 28, 2019

View reviewed changes

setup.cfg Show resolved Hide resolved

nicoddemus force-pushed the drop-py27-py34 branch from 6969afb to e6f81d2 Compare May 29, 2019 00:36

asottile reviewed May 29, 2019

View reviewed changes

nicoddemus commented May 29, 2019

View reviewed changes

src/_pytest/compat.py Outdated Show resolved Hide resolved

nicoddemus commented May 29, 2019

View reviewed changes

src/_pytest/compat.py Show resolved Hide resolved

blueyed requested review from blueyed and removed request for blueyed May 29, 2019 22:21

nicoddemus commented May 29, 2019

View reviewed changes

.travis.yml Show resolved Hide resolved

auvipy reviewed May 30, 2019

View reviewed changes

nicoddemus changed the base branch from features to master June 2, 2019 14:45

nicoddemus force-pushed the drop-py27-py34 branch from b70f370 to 7c784b7 Compare June 2, 2019 14:48

asottile approved these changes Jun 2, 2019

View reviewed changes

doc/en/example/markers.rst Outdated Show resolved Hide resolved

nicoddemus force-pushed the drop-py27-py34 branch from 7c784b7 to bdc9729 Compare June 2, 2019 15:14

nicoddemus changed the title ~~[WIP] Drop py27 and py34 support~~ Drop py27 and py34 support Jun 2, 2019

nicoddemus force-pushed the drop-py27-py34 branch 2 times, most recently from 9cd23ab to e90534d Compare June 2, 2019 15:57

RonnyPfannschmidt reviewed Jun 2, 2019

View reviewed changes

nicoddemus force-pushed the drop-py27-py34 branch from e90534d to 4d49ba6 Compare June 2, 2019 17:40

RonnyPfannschmidt approved these changes Jun 2, 2019

View reviewed changes

nicoddemus merged commit 602a290 into pytest-dev:master Jun 2, 2019

nicoddemus deleted the drop-py27-py34 branch June 2, 2019 22:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop py27 and py34 support #5318

Drop py27 and py34 support #5318

nicoddemus commented May 27, 2019 •

edited

Loading

codecov bot commented May 27, 2019 •

edited

Loading

nicoddemus commented May 28, 2019

blueyed commented May 28, 2019

nicoddemus commented May 28, 2019

blueyed commented May 28, 2019

asottile commented May 28, 2019

blueyed commented May 28, 2019

asottile commented May 28, 2019

blueyed commented May 28, 2019

asottile commented May 28, 2019

nicoddemus commented May 28, 2019

asottile commented May 28, 2019

nicoddemus commented May 28, 2019

nicoddemus commented May 29, 2019

asottile left a comment

asottile May 29, 2019

nicoddemus commented May 29, 2019

nicoddemus commented May 29, 2019

auvipy left a comment

nicoddemus commented Jun 2, 2019

asottile left a comment

asottile commented Jun 2, 2019

RonnyPfannschmidt Jun 2, 2019

nicoddemus Jun 2, 2019 •

edited

Loading

RonnyPfannschmidt Jun 2, 2019

nicoddemus Jun 2, 2019

nicoddemus commented Jun 2, 2019

Drop py27 and py34 support #5318

Drop py27 and py34 support #5318

Conversation

nicoddemus commented May 27, 2019 • edited Loading

codecov bot commented May 27, 2019 • edited Loading

Codecov Report

nicoddemus commented May 28, 2019

blueyed commented May 28, 2019

nicoddemus commented May 28, 2019

blueyed commented May 28, 2019

asottile commented May 28, 2019

blueyed commented May 28, 2019

asottile commented May 28, 2019

blueyed commented May 28, 2019

asottile commented May 28, 2019

nicoddemus commented May 28, 2019

asottile commented May 28, 2019

nicoddemus commented May 28, 2019

nicoddemus commented May 29, 2019

asottile left a comment

Choose a reason for hiding this comment

asottile May 29, 2019

Choose a reason for hiding this comment

nicoddemus commented May 29, 2019

nicoddemus commented May 29, 2019

auvipy left a comment

Choose a reason for hiding this comment

nicoddemus commented Jun 2, 2019

asottile left a comment

Choose a reason for hiding this comment

asottile commented Jun 2, 2019

RonnyPfannschmidt Jun 2, 2019

Choose a reason for hiding this comment

nicoddemus Jun 2, 2019 • edited Loading

Choose a reason for hiding this comment

RonnyPfannschmidt Jun 2, 2019

Choose a reason for hiding this comment

nicoddemus Jun 2, 2019

Choose a reason for hiding this comment

nicoddemus commented Jun 2, 2019

nicoddemus commented May 27, 2019 •

edited

Loading

codecov bot commented May 27, 2019 •

edited

Loading

nicoddemus Jun 2, 2019 •

edited

Loading