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

Remove unused import (from ruff . --fix) #2917

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Remove unused import (from ruff . --fix) #2917

merged 2 commits into from
Oct 20, 2023

Conversation

Andrew-S-Rosen
Copy link
Contributor

@Andrew-S-Rosen Andrew-S-Rosen commented Oct 16, 2023

Description

I applied ruff autofixes (i.e. ruff . --fix) to the source code, which found... precisely one unused import. 😅

Type of change

  • Code maintentance/cleanup

@Andrew-S-Rosen Andrew-S-Rosen changed the title Apply ruff autofixes Remove unused import (from ruff . --fix) Oct 16, 2023
@benclifford
Copy link
Collaborator

i feel like this removed a lot more than it should have

@Andrew-S-Rosen
Copy link
Contributor Author

Andrew-S-Rosen commented Oct 16, 2023

@benclifford: There should only be one line in one file changed. You probably saw an older commit where I also ran ruff on the test suite, which wreaked havoc on things. I reverted that change.

@benclifford
Copy link
Collaborator

the existing tooling usually catches unused imports - it's interesting that it didn't catch this one

@benclifford benclifford merged commit c788de6 into Parsl:master Oct 20, 2023
4 checks passed
@Andrew-S-Rosen Andrew-S-Rosen deleted the ruff branch October 20, 2023 18:04
@benclifford
Copy link
Collaborator

I realised why the one file this PR ended up modifying wasn't picked up by flake8: it's because this file isn't source code, but instead output code generated by poncho_package_serverize and committed into the Parsl tree (see PR #2605).

It makes less sense to edit machine generated object code, even though it happens to be in Python syntax - this isn't a file for humans to be editing, and changes to that file should be via changing how poncho_package_serverize creates that file, or by editing the source file parsl_coprocess_stub.py.

In a more pure build system, this file wouldn't exist in version control and would be built from the stub file at compile/build time - but that isn't what's happening, so this file ends up as a violation of "don't commit object code into version control" instead.

@Andrew-S-Rosen
Copy link
Contributor Author

Ah, well now that clears things up!

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