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

Run pyupgrade to update Python syntax used #2043

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

ni-jfitzger
Copy link
Collaborator

@ni-jfitzger ni-jfitzger commented Apr 1, 2024

  • This contribution adheres to CONTRIBUTING.md.

  • [ ] I've updated CHANGELOG.md if applicable.

  • [ ] I've added tests applicable for this pull request

What does this Pull Request accomplish?

Note: No hand-coded changes are included.

Using a script, I ran pyupgrade --py38-plus on files in nimi-python. pyupgrade is a tool used to automatically update the syntax used in Python code to syntaxes available in newer versions.

The script ran on the following folders:

  • build
  • docs
  • source
  • tools

The script ignored folder paths containing:

  • .tox
  • __pycache__

The script ignored files with the extensions:

  • ..digiproj
  • .mak
  • .proto

The script ignored imported metadata files (the ones that say they were generated) as well as "fancy_fetch_capture_waveform.py.mako" (pyupgrade messed up the indentation) and "_get_error_description.py.mako" (pyupgrade messed up the indentation).

List of automatic changes that I noticed:

  • String formatting (most of the changes were this)
    • Use fstrings where reasonable
    • delete unnecessary index arguments to {} for strings where we continue to use .format
      • pyupgrade is timid about f-string usage in order to maintain short and readable code
    • replace use of really old % format specifier with .format
  • removal of coding: utf-8 comment (it's the default encoding)
  • drop unnecessary Object argument from base class declarations
  • Use dict comprehension rather than whatever we were doing in nifake/unit_tests/test_grpc.py
  • Use set comprehension in metadata_add_all.py
  • delete unnecessary arguments from open() calls

I discarded changes to test_converters.py. I believe we're not passing the right arguments for some tuple tests, so I want to handle that separately.

List issues fixed by this Pull Request below, if any.

None

What testing has been done?

  • I reviewed every line that was changed.
  • PR Checks

…ividual files:

- Use fstrings where reasonable
- replace use of really old % format specifier
- removal of coding: utf-8 comment (it's the default encoding)
- drop unnecessary Object argument from base class declarations
- delete leading u from unicode-literals (python 3 strings support unicode characters)
- delete unnecessary index arguments to {} for strings where we continue to use .format
  - pyupgrade is timid about f-string usage in order to maintain short and readable code
- Use dict comprehension rather than whatever we were doing in nifake/unit_tests/test_grpc.py
- delete unnecessary arguments from open() calls
- Use set comprehension in metadata_add_all.py

Discarded changes to test_converters.py. I believe we're not passing the right arguments for some tuple tests
In addition to reflecting the changes in the previous commit, the copyright date in conf.py was updated to include 2024.
@ni-jfitzger
Copy link
Collaborator Author

I considered cleaning up some of the remaining .format() calls, but decided to leave this as only automated changes.

@ni-jfitzger
Copy link
Collaborator Author

pyupgrade can be used as a pre-commit hook, but I don't think that would be a good idea, given that it broke the indentation of some of our templates.

@ni-jfitzger ni-jfitzger marked this pull request as ready for review April 2, 2024 13:10
@marcoskirsch marcoskirsch merged commit b399fea into ni:master Apr 2, 2024
35 checks passed
@ni-jfitzger ni-jfitzger deleted the run-pyupgrade branch April 2, 2024 14:08
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