-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fixing bugs from pr41 #42
Conversation
….com:valeriupredoi/bgcval2 into improving_the_timeseries_variables_interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but pls nuke commented out code or restore it if needed 🍺
# 'convert': ukp.NoChange, | ||
# 'units': 'pH', | ||
# } | ||
# else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls remove commented out code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole section is in the 4000 lines that are scheduled to be deleted over the next few PRs. I'll remove it then!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the more you remove (obv if needs to be removed) the less spaghetti the code becomes so your next PR will be cleaner to solve :)
This branch fixes a couple bugs introduced in PR 41, when the
analysisSuite
variable was renamedsuites
inanalysis_timeseries.py
.