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

chore: requirement updates #50

Merged
merged 2 commits into from
Jun 22, 2022
Merged

Conversation

rayzhou-bit
Copy link
Contributor

@rayzhou-bit rayzhou-bit commented Jun 16, 2022

Ran the following commands locally:
make upgrade
make test

make upgrade updated the various requirements to the latest version. This introduced various errors and warnings when running make test.

I corrected the following pylint errors and warnings:
E0015 (unrecognized-option)
W0012 (unknown-option-value)
R0022 (useless-option-value)
C3001 (unnecessary-lambda-assignment)
E4702 (modified-iterating-dict)

@connorhaugh
Copy link

Can you inform as to what you did to verify these upgrades?

@rayzhou-bit
Copy link
Contributor Author

I ran make upgrade locally which seems to be what the CI was trying to do. It's not passing for tests right now though. Trying to figure this out.

@bszabo
Copy link

bszabo commented Jun 17, 2022

Could you please enter a comment in this PR that documents which dependency's change you're responding to, what the nature of the breaking change was, and your general approach to resolving it? If the issue is that pylint became more picky about what constitutes "good python," then perhaps a comment about what types of expressions are no longer OK.

@rayzhou-bit
Copy link
Contributor Author

@bszabo I believe everything that broke was related to pylint. I went through the test/lint errors one by one. A large number of the them were deprecated options. For other errors, I modeled code changes after examples in pylint documentation, such as https://pylint.pycqa.org/en/latest/user_guide/messages/convention/unnecessary-lambda-assignment.html . I could have had them ignored, but it feels like it would be better to update.

@rayzhou-bit rayzhou-bit merged commit 10d09f6 into master Jun 22, 2022
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.

3 participants