-
Notifications
You must be signed in to change notification settings - Fork 36
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
xnemogcm #155
Comments
Editor in Chief checksHi there! Thank you for submitting your package for pyOpenSci Please check our Python packaging guide for more information on the elements
Editor comments |
Hi @ocefpaf, I can directly mention that some requirements are not fulfilled:
As far as I can say, the other requirements should be fulfilled. |
That's awesome! Here are a few minor comments/questions I have after the first pass:
|
Maybe the README is not so clear, the github is mirrored to gitlab (not from gitlab). The reason is that I do not want to force anyone to use github. But I could remove the sentence and keep my email address only.
No, I did not publish the doc anywhere. I started all the readthedoc machinery, but realised that having only these few examples was sufficient. As this packages has been developed only by me, I also had a lot to learn and a proper website for the doc was not urgent. I can work on a proper website for the doc during this review process. I do not aim to change the content of the examples (unless I get comments from the reviewers), so I think the review can start without this implemented yet.
|
I understand. By making a choice we always end up making some folks unhappy. In my experience, mirroring like that always brings headaches in the long run. With that said, this is not important for your application here and you don't need to change that for PyOS.
@rcaneill we usually only start the review after all those points are checked. The reason is b/c the reviewers will read those docs and give you some feedback. Let me know if you need help setting docs, examples, etc in your repo. Once we are done I'll find two reviewers for your project and start the review process. |
Ok I understand, I will try to check all boxes in the following weeks! |
Hi @ocefpaf , I updated the xnemogcm github repository, everything should be checked now! :D |
@rcaneill there seems to be a documentation build problem in page https://xnemogcm.readthedocs.io/en/latest/examples/recombing_mesh_mask_domain_cfg/ Also, some of your syntax there is not Windows friendly and users that copy-n-paste may find themselves with errors. Those are not blockers for us to continue but I wanted to give that feedback here before I forget. We are now looking for 2 reviewers to start the process. Thank you! |
thanks for catching this, it is now corrected.
Are you speaking about the |
That and I believe some of the paths may not be parsed correctly. I don't have a Windows machine to test in |
pathlib.Path is handling the windows conversion: https://stackoverflow.com/questions/2953834/how-should-i-write-a-windows-path-in-a-python-string-literal/68234511#68234511 For the |
👋 Hi Callum Rollo and The following resources will help you complete your review:
Please get in touch with any questions or concerns! Your review is due: March 5th, 2024 (3 weeks). Reviewers: @callumrollo and @paigem PS: You can comment, open issue, and/or PR directly in the xnemogcm for this review. Just make sure to cross-reference them here. |
Hej Romain, I'm starting my review today. The package looks really good! I see you have an Issue in to rename master >> main. Perhaps now is a good time to do that before I queue up a couple of small PRs rcaneill/xnemogcm#72 |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Readme file requirements
The README should include, from top to bottom:
NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)
UsabilityReviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Functionality
For packages also submitting to JOSS
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted. The package contains a
Final approval (post-review)
Estimated hours spent reviewing: 5 hours Review Comments
|
Hi Calum, thanks for your time :), I updated the name of the default branch to |
Thanks @callumrollo for all the PR and improvements. @ocefpaf am I supposed to check / merge the PR on the fly, or wait for the end of the review? |
Up to you. Sometimes the second reviewer can add upon the first one and waiting makes sense. Sometimes merging/solving everything now makes it easier b/c it reduces the context for the second review. |
Ok I'll try to merge on the way then |
I've just about completed my review of the package. It looks to be in good shape! I particularly appreciate the comprehensive test suite. I've just raised one last issue with a missing docstring. I don't work with nemo data, but using the example data provided I have tested that the functions in xnemogcm work as described. Hopefully @paigem can comment with some more domain specific expertise. |
Thanks @callumrollo for you time and comments, I really appreciate. |
If this package isn't being submitted to JOSS I believe my review is complete. Thanks @rcaneill for the speedy response to Issues and PRs |
Thanks for your quick updates @rcaneill - it all looks great! I'm continuing to go through my review checklist and have a couple questions:
I am continuing through the checklist and will reach out with any further questions. |
I added CITATION.cff file in rcaneill/xnemogcm#104 |
Thanks @paigem for your time, and for your great suggestions! I really appreciate. |
@rcaneill please let me know when you are done parsing the suggestions and please both reviewers for their +1 so we can move this application forward. |
@ocefpaf I implemented / merged / commented all of their suggestions. So for me, we can move forward :) |
Awesome! @paigem and @callumrollo can I get your +1 before we move to the next stage? |
@ocefpaf lgtm! |
Yes, everything looks great from my end! 😊 |
@rcaneill I'm moving tomorrow and will be offline for a few days. I'll wrap this one up as soon as I get settle in our new place. If you don't hear from in ~1 week, please ping me again! |
🎉 xnemogcm has been approved by pyOpenSci! Thank you Romain Caneill (@rcaneill) for submitting xnemogcm and many thanks to Paige Martin (@paigem) and Callum Rollo (@callumrollo) for reviewing this package! 😸 Author Wrap Up TasksThere are a few things left to do to wrap up this submission:
Editor Final ChecksPlease complete the final steps to wrap up this review. Editor, please do the following:
If you have any feedback for us about the review process please feel free to share it here. We are always looking to improve our process and documentation in the peer-review-guide. |
@rcaneill do you need help finalizing the 4 items in #155 (comment)? Also, if you have some free time can you fill out our maintainers survey at https://forms.gle/BLGVCfUdiJHS5YJY7? |
BTW, @rcaneill ! We want to invite you and your maintainer team to write a blog post (totally optional) on your package for us to promote your work! if you are interested - here are a few examples of other blog posts: and here is a markdown example that you could use as a guide when creating your post. it can even be a tutorial like post that highlights what your package does. then we can share it with people to get the word out about your package. If you are too busy for this no worries. But if you have time - we'd love to spread the word about your package! |
Last but not least, do you (@rcaneill, @paigem, and @callumrollo) would like to be invited to PyOS slack? It will help you share your experience, get in touch with the community, and learn from others. |
Hi @ocefpaf , I am on parental leave until Monday, I'll take time next week to answer you in details. |
Enjoy! I'm on forced* parental leave too so that is convenient for both of us 😄 (*Waiting for our day care application to be accepted so we can get the little one in school at the new home.) |
sure! |
The new xnemogcm version is 0.4.3, the doi is: https://doi.org/10.5281/zenodo.10973846 Thanks again @ocefpaf , @callumrollo, and @paigem for the review |
Thanks for the invite, but I prefer to not use Slack. I am still available by email for future exchanges. |
I will see if I have time for it. If so, should I open a PR on https://github.com/pyOpenSci/pyopensci.github.io? |
Yes. You can see an example here. |
I'm closing this as complete. Thanks again for the reviewers and for @rcaneill for your patience! |
as discussed in #154 I make a full submission
Submitting Author: Romain Caneill (@rcaneill)
All current maintainers: (@rcaneill)
Package Name: xnemogcm
One-Line Description of Package: Interface to open NEMO global circulation model output dataset with xarray and create a xgcm grid.
Repository Link: https://github.com/rcaneill/xnemogcm/
Version submitted: 0.4.2
Editor: @ocefpaf
Reviewer 1: @paigem
Reviewer 2: @callumrollo
Archive:
Version accepted: 0.4.3
JOSS DOI: N/A
Date accepted (month/day/year): 04/02/2024
Code of Conduct & Commitment to Maintain Package
Description
xnemogcm is an interface to open NEMO ocean global circulation model output dataset and create a xgcm grid. NEMO 3.6, 4.0, and 4.2.0 are tested and supported. It can handle large simulations, is aware of meshgrid files, and makes it easy to handle the netCDF outputs od NEMO.
Scope
Please indicate which category or categories.
Check out our package scope page to learn more about our
scope. (If you are unsure of which category you fit, we suggest you make a pre-submission inquiry):
Domain Specific & Community Partnerships
Community Partnerships
If your package is associated with an
existing community please check below:
Pangeo
For all submissions, explain how the and why the package falls under the categories you indicated above. In your explanation, please address the following points (briefly, 1-2 sentences for each):
xnemogcm extracts the NEMO output and adds metadata (+ do some other things as change coordinate names, etc), and produces new xarray datasets. It is thus data processing.
The target audience is anyone working with NEMO outputs and python. The scientific applications are any analyse that one want to do with NEMO outputs.
Yes, xorca. My package differs as 1) it is more recent, updated to work with newer versions of NEMO, and 2) it uses a very different method to sort variables on the proper grid point (center, face, etc) (cf https://xgcm.readthedocs.io/en/latest/grids.html). xorca uses hardcoded variables, while xnemogcm uses attributes (either output directly by NEMO, or they can also be given while calling the processing functions).
@tag
the editor you contacted:I made a pre-submission enquiry, issue #154
Technical checks
For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:
Publication Options
JOSS Checks
paper.md
matching JOSS's requirements with a high-level description in the package root or ininst/
.Note: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.
Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?
This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.
Confirm each of the following by checking the box.
Please fill out our survey
submission and improve our peer review process. We will also ask our reviewers
and editors to fill this out.
P.S. Have feedback/comments about our review process? Leave a comment here
Editor and Review Templates
The editor template can be found here.
The review template can be found here.
The text was updated successfully, but these errors were encountered: