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

Pep8 linting #107

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Pep8 linting #107

wants to merge 8 commits into from

Conversation

ledm
Copy link
Collaborator

@ledm ledm commented Aug 10, 2023

Ran a linter, and a few refactors over bgcval2.

Code should be cleaner and more readable, but a lot of changes to review.

Happy to do it in stages.

Closes #106

@valeriupredoi
Copy link
Owner

@ledm thanks to you we now have a "standards" label, although am a bit scared to look at this PR just yet 🤣

@valeriupredoi
Copy link
Owner

@ledm this is starting to become stale, mate. You reckon we can just power on sometime and get it over the finish line? I can help with it since I have to yapf a whole lot of code for the ANCIL project too

@ledm ledm marked this pull request as ready for review November 7, 2023 14:35
Conflicts:
	bgcval2/analysis_compare.py
@@ -1635,34 +1697,8 @@ def newImageLocation(fn):
print("Adding ", relfn, "to script")

if len(otherFilenames):
# I think this is never happens anymore.
# This should never happens.
assert 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
assert 0
print('Report: otherFilenames is too long:', otherFilenames )
raise IndexError

@ledm
Copy link
Collaborator Author

ledm commented Nov 7, 2023

I think the tests are currently failing because they reference some of the objects that were renamed in this PR. I'll see what I can do.

Edit. nope, jsut me being stupid.

@valeriupredoi
Copy link
Owner

nice! Then you reckon is close to merge? I can plop a flake8 test then - that'll say "look how standardy and PEPy this package is"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pep8 non-compliance and minor bugs.
2 participants