Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

new QVP functions #807

Closed
wants to merge 2 commits into from
Closed

Conversation

meteoswiss-mdr
Copy link
Contributor

Dear Py-ART masters,

I submit here a bunch of functions to compute a family of QVP-like products. They are stored in a radar-like object.

You can find the new functions in pyart/retrieve/qvp.py

Greetings from Switzerland!

@zssherman
Copy link
Collaborator

Hi @meteoswiss-mdr , thank you for the pull request! I will be able to take a look next week, but I reran travis for now. Looks like it failed on their end.

@meteoswiss-mdr
Copy link
Contributor Author

Thank you. Let me know if there are any issues.

@meteoswiss-mdr
Copy link
Contributor Author

@zssherman, any news on this one?

@scollis
Copy link
Member

scollis commented Feb 27, 2019 via email

@meteoswiss-mdr
Copy link
Contributor Author

Sorry to hear that! I hope he is fine now. Let me know if you need any clarification

@zssherman
Copy link
Collaborator

@meteoswiss-mdr I apologize for the delay. With the release done, I now can review these enhancements. Thank you for the pull request! I will start reviewing it.

@zssherman
Copy link
Collaborator

@meteoswiss-mdr Just a few comments, for functions such as evp, rqvp etc, should the variables in the functions themselves and docstrings reflect evp rqvp, so under return for examples for rqvp, instead of saying qvp, have rqvp? Other comment I had was such a function like compute directional stats, maybe get moved to the utils sub module? @scollis Would you agree?

@meteoswiss-mdr
Copy link
Contributor Author

Hi @zssherman,

I agree on both points.

@zssherman
Copy link
Collaborator

@meteoswiss-mdr I currently am doing some changes and will submit a pull request to your branch. Do you possibly have unit tests for these new functions?

@meteoswiss-mdr
Copy link
Contributor Author

Hi @zssherman,

I do not have unit tests. I tested them by having a look on some processed data.

I will see if I can have a look at your proposed changes next week.

@zssherman zssherman closed this May 3, 2019
@zssherman zssherman reopened this May 3, 2019
@zssherman
Copy link
Collaborator

Fix conflict, working on how to approach unit testing.

@zssherman
Copy link
Collaborator

Actually going to close and create a new PR.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants