Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

really drop python<=3.7 support #399

Closed
wants to merge 2 commits into from
Closed

Conversation

kloczek
Copy link

@kloczek kloczek commented Apr 17, 2024

Filter all code over pyupgrade --py38-plus.

Filter all code over `pyupgrade --py38-plus`.

Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
@kloczek
Copy link
Author

kloczek commented Apr 17, 2024

My PR not messed with that line on which point plint
image

Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
@brunato
Copy link
Member

brunato commented Apr 17, 2024

Sorry but too many changes in so many files to be merged in the master branch.
Also I agree with some changes (e.g. set() constructors) but not others (e.g. C-style formatted strings replaced by format() in some cases).

I could agree for a merge in the develop branch for a limited set of cases (set, super, ...).

Thank you

@kloczek
Copy link
Author

kloczek commented Apr 17, 2024

Sorry but too many changes in so many files to be merged in the master branch.

All those changes has been generated automatically by pyupgrade --py38-plus.
You can install pyupgrade and execute find . -name\*.py | xargs pyupgrade --py38-plus and you will have exactly the same patch.

@brunato
Copy link
Member

brunato commented Apr 18, 2024

Sorry but too many changes in so many files to be merged in the master branch.

All those changes has been generated automatically by pyupgrade --py38-plus. You can install pyupgrade and execute find . -name\*.py | xargs pyupgrade --py38-plus and you will have exactly the same patch.

I understood that, but in development, not in master branch.

I saw that pyupgrade doesn't have so many exception options, anyway i will try it during development.

Dropping Python 3.7 doesn't mean necessarily that all the codebase is Py38+. In this case the compatibility is dropped mainly by the usage of protocols defined in the base package elementpath.

Thank you

@brunato brunato closed this Apr 18, 2024
@kloczek
Copy link
Author

kloczek commented Apr 18, 2024

OK I understand.
You know what to do and how.
Just please do that when you will have time.

@brunato
Copy link
Member

brunato commented Apr 19, 2024

Tested on develop. Two things:

  1. The code produced with pyupgrade --py38-plus is exactly the same produced with pyupgrade --py37-plus
  2. Some strings are not translated neither if it's possible, maybe pyupgrade gets confused sometimes when the value includes quotes or round brackets.

@brunato
Copy link
Member

brunato commented Apr 19, 2024

Also found a bug applying to a module in elementpath. A for-yield is replaced with a yield from despite the variable is an instance attribute:

diff --git a/elementpath/xpath_context.py b/elementpath/xpath_context.py
index 3b16ee9..483bb85 100644
--- a/elementpath/xpath_context.py
+++ b/elementpath/xpath_context.py
@@ -386,8 +386,7 @@ class XPathContext:
             status = self.item, self.axis
             self.axis = 'attribute'
 
-            for self.item in self.item.attributes:
-                yield self.item
+            yield from self.item.attributes
 
             self.item, self.axis = status

@kloczek
Copy link
Author

kloczek commented Apr 19, 2024

Also found a bug applying to a module in elementpath. A for-yield is replaced with a yield from despite the variable is an instance attribute:

Just open the ticket in pyupgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants