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

Update make_extrema_longrun_3day.py #567

Merged
merged 1 commit into from
Aug 10, 2018

Conversation

mfwehner
Copy link
Contributor

Revision by MFW 08/10/2018: Removed the 4 seasons for speed. Added some comments

# Revision by MFW 08/10/2018: Removed the 4 seasons for speed. Added some comments
@mfwehner mfwehner closed this Aug 10, 2018
@mfwehner mfwehner reopened this Aug 10, 2018
@gleckler1 gleckler1 merged commit 30923df into PCMDI:534_pjg_mfw_extremes Aug 10, 2018
@gleckler1
Copy link
Contributor

@mfwehner Great, that works fine.... was not sure you had permission.

@durack1
Copy link
Collaborator

durack1 commented Aug 10, 2018

@mfwehner welcome to team PCMDI!

@doutriaux1
Copy link
Contributor

@mfwehner a few comments

  • please use the pmpparser instead of sys.argv.
  • add a test
  • use autopep8 and flake8 for code niceness

If you need help with any of this just let me know.

@mfwehner
Copy link
Contributor Author

mfwehner commented Aug 20, 2018 via email

@doutriaux1
Copy link
Contributor

@mfwehner ok I will write a notebook about this and add it to the doc (developer) it is a nice addition. will post link here as well.

@doutriaux1
Copy link
Contributor

@gleckler1 we might already have pmp parser examples as part of cia can you post the link please?

@gleckler1
Copy link
Contributor

@mfwehner @doutriaux1 We need to think about the best use of MFW's time. I am already working with his make_extrema_longrun_3day.py and pentadal scripts which are working but very slow. CD, MFW is not familiar with python dictionaries and other functionality we heavily rely on. We can't expect him to be able to follow our param file and agrparse conventions until we have that thoroughly documented. That is not going to be available until mid September. PG checking out on 2wk VAC this Weds, 8/22.

@doutriaux1
Copy link
Contributor

@gleckler1 ok. @mfwehner I can review your additions in the mean time.

@mfwehner
Copy link
Contributor Author

mfwehner commented Aug 20, 2018 via email

@durack1
Copy link
Collaborator

durack1 commented Aug 20, 2018

@mfwehner you just did, by replying to email - although it's cleaner to reply to the issue thread (less cruft)

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.

None yet

4 participants