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

Reduce flake8 max-line-length #3122

Merged
merged 3 commits into from
Mar 4, 2024
Merged

Conversation

Harichandra-Prasath
Copy link
Contributor

Description

Reduced flake-8 maximum length and changed some lines which violate the new configuation

Changed Behaviour

This pr wont change any behaviour of the project

Fixes

Fixes #3105

Type of change

  • Code maintenance/cleanup

@Harichandra-Prasath
Copy link
Contributor Author

May i know why the tests are failing ? I ran local tests after the change and it went fine. Should i have to fix it so that it will pass these tests?

@benclifford
Copy link
Collaborator

your PR failed in CI, with this error from mypy, the static type checking tool we used:

parsl/executors/workqueue/executor.py:65: error: List or tuple literal expected as the second argument to "namedtuple()"  [misc]

That's because you can't use a variable as a parameter to namedtuple when doing static typechecking (even if the variable is a constant on the line before...)

You should be able to recreate this error yourself in your development environment by running make mypy and see that it happens in your own environment.

Then some different ideas:

idea 1: You can split the string into multiple lines directly in the call to namedtuple - for example, like this:

def f(x: str):
 print(x)


f('a'
  'b')

idea 2, if we knew all the types of each piece of the named tuple, we could use a different syntax that I think is much nicer - but you would have to know all the types, so perhaps that is something to try in a later pull request: https://docs.python.org/3/library/typing.html#typing.NamedTuple

@benclifford benclifford merged commit e364791 into Parsl:master Mar 4, 2024
6 checks passed
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.

Outreachy: getting familiar with the development environment
2 participants