selenium icon indicating copy to clipboard operation
selenium copied to clipboard

Published 20 hours ago • verified publisher icon SeleniumHQ

really drop python<=3.7 support

Open kloczek opened this issue 9 months ago • 6 comments

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

  • [x] I have read the contributing document.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] 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

    kloczek avatar May 15 '24 23:05 kloczek

    CLA assistant check
    All committers have signed the CLA.

    CLAassistant avatar May 15 '24 23:05 CLAassistant

    PR Description updated to latest commit (https://github.com/SeleniumHQ/selenium/commit/363512f585e4f8da63eaf662f033693b8b353817)

    codiumai-pr-agent-pro[bot] avatar May 15 '24 23:05 codiumai-pr-agent-pro[bot]

    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

    codiumai-pr-agent-pro[bot] avatar May 15 '24 23:05 codiumai-pr-agent-pro[bot]

    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

    codiumai-pr-agent-pro[bot] avatar May 16 '24 00:05 codiumai-pr-agent-pro[bot]

    Signed

    kloczek avatar May 16 '24 09:05 kloczek

    @kloczek this is not passing our lint tests, can you please update it?

    titusfortner avatar Jun 18 '24 20:06 titusfortner