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.

Sign up for GitHub

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

Jump to bottom

Update, Apply, and Automate Python Linting #311

Merged
merged 56 commits into from
Jul 20, 2023
Merged

Update, Apply, and Automate Python Linting #311

merged 56 commits into from
Jul 20, 2023

Conversation

ankona
Copy link
Contributor

@ankona ankona commented Jun 30, 2023 •
edited
Loading

This PR mitigates linting issues that have slipped in over time. It includes the following subtasks:

  • Updates pylint configuration to newer format.
  • Applies pylint as a step in CI workflow (initially non-blocking)
  • Mitigates as many issues as possible
  • Where mitigation is not possible (API break, etc), disable linter.

@ankona ankona added the API break Issues that include incompatible API changes label Jul 3, 2023
@ankona ankona force-pushed the oss339 branch 4 times, most recently from 1d91e10 to 2beeeb5 Compare July 5, 2023 23:00
@ankona ankona marked this pull request as ready for review July 6, 2023 15:28
@ankona ankona requested review from MattToast, ashao and mellis13 July 6, 2023 15:30
@ankona ankona changed the title Oss339 Update, Apply, and Automate Python Linting Jul 10, 2023
@codecov
Copy link

codecov bot commented Jul 10, 2023 •
edited
Loading

Codecov Report

Merging #311 (8278872) into develop (c7eb6d4) will increase coverage by 0.18%.
The diff coverage is 83.59%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #311      +/-   ##
===========================================
+ Coverage    87.00%   87.19%   +0.18%     
===========================================
  Files           59       59              
  Lines         3518     3522       +4     
===========================================
+ Hits          3061     3071      +10     
+ Misses         457      451       -6
Impacted Files Coverage Δ
smartsim/__init__.py 100.00% <ø> (ø)
smartsim/_core/config/config.py 98.41% <ø> (ø)
smartsim/_core/launcher/launcher.py 100.00% <ø> (ø)
smartsim/_core/launcher/step/localStep.py 88.57% <ø> (ø)
smartsim/_core/launcher/step/mpiStep.py 40.90% <0.00%> (ø)
smartsim/_core/launcher/util/launcherUtil.py 100.00% <ø> (ø)
smartsim/_core/utils/network.py 100.00% <ø> (ø)
smartsim/log.py 75.00% <0.00%> (-2.42%) ⬇️
smartsim/ml/tf/__init__.py 100.00% <ø> (ø)
smartsim/ml/torch/data.py 83.92% <ø> (ø)
... and 32 more

billschereriii
billschereriii reviewed Jul 11, 2023
View reviewed changes
@ankona
Copy link
Contributor Author

ankona commented Jul 19, 2023

rebased feature branch on latest develop (post-CLI refactor)

@ankona ankona added area: third-party Issues related to Issues related to dependencies and third-part and third-party package integrations area: CI/CD Issues related to continuous integration and deployment type: usability Issues related to ease of use and removed API break Issues that include incompatible API changes labels Jul 19, 2023
@ankona
Copy link
Contributor Author

ankona commented Jul 19, 2023

@mellis13 @billschereriii could you rescind any requests for changes if those things have been met?

mellis13
mellis13 approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR -- thank you! LGTM!

billschereriii
billschereriii approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@billschereriii billschereriii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for all the hard work on this one

@ankona ankona merged commit 211268e into CrayLabs:develop Jul 20, 2023
@ankona ankona deleted the oss339 branch July 24, 2023 14:25
@ankona ankona restored the oss339 branch July 24, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mellis13 mellis13 mellis13 approved these changes

@billschereriii billschereriii billschereriii approved these changes

@MattToast MattToast Awaiting requested review from MattToast

@ashao ashao Awaiting requested review from ashao

Assignees
No one assigned
Labels
area: CI/CD Issues related to continuous integration and deployment area: third-party Issues related to Issues related to dependencies and third-part and third-party package integrations type: usability Issues related to ease of use
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ankona @mellis13 @billschereriii