-
Notifications
You must be signed in to change notification settings - Fork 33
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 series.py #485
Update series.py #485
Conversation
Fixes part-2 of issue: LinkedEarth#484 Move ntau-pop up to avoid errors where ntau does not exist. Add condition for the scenario where 'tau' is in settings, (as is the case when signif_test() calls wavelet_coherence()).
Fixed minor string issue.
Hi @neillinehan thanks for suggesting this fix. Did you end up creating a pytest for it as well? This is something we encourage for any change to the code, and you already have all it takes to make one. Another thing is that you need to nominate someone to review your code. I think @fzhu2e would be best if he has time, as this functionality is his baby, and he knows that part of the code the best. |
Hi @CommonClimate, The proposed fix works in my local Jupyter notebook environment. I’ve tested the changes informally and everything works as expected. I'm new to GitHub and finding the process of writing formal tests a bit time-consuming at the moment, I understand if this means my current contribution cannot be accepted. I may return to this at a later time. I'm exploring seasonal changes in some crop light responses sampled at a fairly high frequency (1-min), so I need higher time-resolution as opposed to the default high spectral-resolution. |
Hi @neillinehan , |
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.
Approved after adding 2 unit tests to ensure that the changes a) don't break anything and b) work as advertised.
addresses issue #484 |
Fixes #484 (comment)
ntau not in wwz_coherence() arguments, so pop it out of the settings dict after calculations are complete.