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

STY: Update package for pre-commit, will add pre-commit action in another PR. #792

Merged
merged 3 commits into from
Feb 1, 2024
Merged

STY: Update package for pre-commit, will add pre-commit action in another PR. #792

merged 3 commits into from
Feb 1, 2024

Conversation

zssherman
Copy link
Collaborator

@zssherman zssherman commented Jan 24, 2024 •
edited
Loading

  • PEP8 Standards or use of linter

@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

AdamTheisen
AdamTheisen approved these changes Jan 29, 2024
View reviewed changes
Copy link
Collaborator

@AdamTheisen AdamTheisen left a comment

Choose a reason for hiding this comment

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

This looks ok from what I can tell. It does seem like it's expanding a lot to one item on a line which takes up a lot more space but I guess is more readable.

@zssherman
Copy link
Collaborator Author

@AdamTheisen Yeah exactly as you stated, it's essentially to make the code more readable

@zssherman
Copy link
Collaborator Author

@AdamTheisen Rerunning the tests as the previous croptype PR should fix this now. Once they pass, want me to merge this one?

@AdamTheisen
Copy link
Collaborator

@zssherman yep, but it looks like the tests are still failing and it's not pulling in the most recent code

@zssherman zssherman closed this Jan 31, 2024
@zssherman zssherman reopened this Jan 31, 2024
@zssherman
Copy link
Collaborator Author

@AdamTheisen Weird, yeah i'll try to reopen the PR, see if that fixes it.

@zssherman zssherman merged commit bbbb767 into ARM-DOE:main Feb 1, 2024
25 of 32 checks passed
@zssherman zssherman deleted the pre-commit branch April 23, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdamTheisen AdamTheisen AdamTheisen approved these changes

@mgrover1 mgrover1 Awaiting requested review from mgrover1 mgrover1 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zssherman @AdamTheisen