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

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Conversation

kloczek
Copy link

@kloczek kloczek commented May 15, 2024

User description

Description

Filter all python code over pupgrade --py38-plus.

Motivation and Context

Make code cleaner.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement


Description

  • Updated Python code to enhance compatibility with Python 3.8+ by removing outdated syntax and improving string formatting.
  • Removed specific mode arguments in file open functions to use default reading mode, simplifying the code.
  • Transitioned to using format method for string formatting across various scripts and modules.

Changes walkthrough 📝

Relevant files
Enhancement
convert_protocol_to_json.py
Simplify file opening in convert_protocol_to_json.py         

common/devtools/convert_protocol_to_json.py

  • Removed the mode argument from open function to use the default mode.
  • +1/-1     
    pdl.py
    Code modernization in pdl.py                                                         

    common/devtools/pdl.py

  • Removed future import for print_function.
  • Updated print syntax to use format method.
  • +1/-2     
    gen_file.py
    Refactor string formatting and file handling in gen_file.py

    javascript/private/gen_file.py

  • Updated string formatting to use format method.
  • Simplified file opening by removing the mode argument.
  • +22/-22 
    webelement.py
    Update type hints in webelement.py                                             

    py/selenium/webdriver/remote/webelement.py

  • Changed return type hint from List to list for Python 3.9+
    compatibility.
  • +1/-1     
    remote_downloads_tests.py
    Simplify file opening in remote_downloads_tests.py             

    py/test/selenium/webdriver/remote/remote_downloads_tests.py

  • Removed the mode argument from open function to use the default mode.
  • +1/-1     
    pinned_browsers.py
    Refactor string formatting in pinned_browsers.py                 

    scripts/pinned_browsers.py

    • Updated string formatting to use format method.
    +17/-17 
    update_copyright.py
    Simplify file opening in update_copyright.py                         

    scripts/update_copyright.py

    • Simplified file opening by removing the mode argument.
    +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Filter all python code over `pupgrade --py38-plus`.
    
    Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
    @CLAassistant
    Copy link

    CLAassistant commented May 15, 2024

    CLA assistant check
    All committers have signed the CLA.

    Copy link

    PR Description updated to latest commit (363512f)

    Copy link

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are mostly syntax updates and string formatting improvements, which are straightforward to review. The PR is well-documented and the changes are consistent across multiple files.

    🧪 Relevant tests

    Yes

    ⚡ Possible issues

    Possible Bug: The removal of the file mode "r" in open() calls might not be an issue since it's the default, but it should be confirmed that no binary files are being read, as this could cause unexpected behavior.

    🔒 Security concerns

    No

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use a context manager for file operations to ensure proper resource management

    Use a context manager to handle file opening and closing to ensure that the file is
    properly closed even if an error occurs during file operations.

    common/devtools/convert_protocol_to_json.py [24-27]

    -input_file = open(file_name)
    -pdl_string = input_file.read()
    -input_file.close()
    +with open(file_name, 'r') as input_file:
    +    pdl_string = input_file.read()
     
    Suggestion importance[1-10]: 8

    Why: This is a valid improvement suggestion as using a context manager for file operations ensures that the file is properly closed even if an error occurs, which is not currently handled in the PR code.

    8
    Enhancement
    Use f-strings for clearer and more efficient string formatting

    Replace the string concatenation in the write method with formatted strings for better
    readability and performance.

    javascript/private/gen_file.py [50]

    -out.write("const {}* const {}[] = {{\n".format(char_type, name))
    +out.write(f"const {char_type}* const {name}[] = {{\n")
     
    Suggestion importance[1-10]: 6

    Why: The suggestion to use f-strings enhances readability and performance, aligning with modern Python practices. This is a good suggestion, but not critical.

    6

    @kloczek
    Copy link
    Author

    kloczek commented May 16, 2024

    Signed

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

    Successfully merging this pull request may close these issues.

    None yet

    2 participants