-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[REVIEW]: pySYD: Automated measurements of global asteroseismic parameters #3331
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @danhey, @benjaminpope it looks like you're currently assigned to review this paper 🎉. Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. ⭐ Important ⭐ If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿 To fix this do the following two things:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
|
👋 @danhey @benjaminpope Thank you so much for agreeing to review! You can find the article in the comment box above ⬆️ , and the checklist and software repository linked in the first comment box on this issue. I think you're good to go -- please let me know if you need anything else. Again, JOSS is an open review process and we encourage communication between the reviewers, the submitting author, and the editor. So please feel free to ask questions at any time, I'm always around. |
Intro Comments:
Installation:
Unit tests:
Repository:
Examples:
Manuscript:
|
Thank you so much @benjaminpope for providing this extremely helpful review so quickly! We also acknowledge the importance of reproducing the paper figure but have been stuck on the best way to implement this. We were wondering if JOSS has a recommended data-hosting website, something analogous to CDS for data releases associated with astronomy papers. Providing the power spectra to reproduce this figure would be about ~300 MB of data and therefore probably too large to provide directly on our GitHub repo. |
Hi all, this has been a pleasure to review! It's a great piece of software, and I have no doubt it will be extremely useful for asteroseismology. Overall, I believe pySYD satisfies the JOSS criteria for publication after a few points have been addressed. I agree with all of Ben's comments, especially regarding the distinction between a command-line vs. API tool. It appears to me that the functionality is there for using pySYD from say, a Jupyter notebook, but this has not been documented. I will try to structure my review in the same format as Ben's. Installation
Unit tests
Repository
Examples and functionality
Manuscript
Only a suggestion, but would it be easier to instead write a small utility function in pySYD that calculates the power spectrum given an input light curve? This would also be useful for the general functionality of PySYD, allowing people to supply a light curve. It would also benefit reproducibility: I have seen a lot of different power spectra normalizations in my time ... |
Agree with all of @danhey's comments. Re @ashleychontos' question about data hosting - there are a few ways to do this... you could just upload a static .ipnyb that implements the required calculations, and describe where the input data come from - I imagine these are programmatically pulled from MAST or KASOC/TASOC. Or you could do it all on git-lfs. Finally, many universities have large, permanent data storage solutions - eg UQ does - you could upload relevant datasets there and link to those. |
@ashleychontos Agree with @benjaminpope. You can provide code that queries and downloads the data and then makes the figure from that downloaded data. Then it is the user's responsibility to store the data (for ~300 MB, not too big an ask), but all they need to do to reproduce the figure is run the provided code. |
👋 @danhey, please update us on how your review is going (this is an automated reminder). |
👋 @benjaminpope, please update us on how your review is going (this is an automated reminder). |
@mbobra apologies in advance since this is all of our (the authors) first time preparing anything like this. First and foremost, what is the most appropriate way to address and comment on either/both of the reviews (similar to a resubmission with AAS)? Following are comments from the reviews that we could use some clarification on before proceeding.
If the above test is not sufficient, we have discussed two other alternatives and would like to hear any comments or suggestions (@danhey included). The first is providing enough examples that cover all optional arguments to make sure no error is produced when they are all called (additionally, a plot could be produced to show the before and after when those arguments are used). The second idea was to have a counter that runs through all functions and if successful, asserts that the number returned equals the number of functions in the pipeline. This would also account for optional functions that are not required for the software to run successfully. |
@ashleychontos thanks! So as a gold standard example, @dfm did this for the If the examples in the documentation match the output verbatim that will be good! It will also be good to have richer output - for example, figures (and autosave figures, rather than click-through) and terminal output, so that we can test that it reproduces the examples exactly. (And a GitHub Action to test one or more examples, even with just a checksum, would meet the continuous testing requirement). |
Agree with @benjaminpope. I also think a good unit test for GitHub Actions would be to just simply run pySYD on a test star for which numax and dnu is well known, and assert that the output of the pipeline is |
Thank you both @benjaminpope and @danhey again for such quick and helpful responses! I will follow Ben's suggestion and open up a separate pull request for each review and update those accordingly. |
👋 @ashleychontos It looks like you already got your answer -- but in any case, I will provide a rather vague response that as long as the reviewers feel (1) you have addressed their concerns and, (2) the software adheres to the JOSS guideline, I will accept the submission! |
@mbobra - can you provide an update about this submission? It seems stuck... |
I am going to pause the review. This gives @ashleychontos time to improve their code and also gives reviewers a break from keeping up with this review thread. @ashleychontos There's nothing wrong (or negative) with pausing a review. If you know you'll be done by a certain time, we can keep it paused. If you're not sure how long all this will take (or if you no longer want to pursue it), you can withdraw this submission and resubmit at a later date. There's nothing wrong with that and it won't impact your next submission. |
Thank you for clarifying @mbobra - the primary three developers for |
|
@editorialbot recommend-accept |
|
|
👋 @openjournals/aass-eics, this paper is ready to be accepted and published. Check final proof 👉📄 Download article If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3690, then you can now move forward with accepting the submission by compiling again with the command |
Congratulations, @ashleychontos 🥳 It was a long ride but I hope you feel the JOSS review process improved the software. I think the final result turned out awesome. The EiC team @openjournals/joss-eics will take it from here! |
Oh absolutely @mbobra I learned more about software and development throughout this process than I ever knew pre-JOSS times combined. Thank you again SO much for everything, especially for riding this out with me! |
@ashleychontos — I've opened a PR with a few formatting edits to the bibliography, but we should be good to go after that: ashleychontos/pySYD#42 |
great @dfm I think it should be all set now! |
@editorialbot recommend-accept |
|
|
👋 @openjournals/aass-eics, this paper is ready to be accepted and published. Check final proof 👉📄 Download article If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3691, then you can now move forward with accepting the submission by compiling again with the command |
@editorialbot accept |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
@danhey, @benjaminpope — many thanks for your reviews here and to @mbobra for editing this submission! JOSS relies upon the volunteer effort of people like you and we simply wouldn't be able to do this without you ✨ @ashleychontos — your paper is now accepted and published in JOSS ⚡🚀💥 |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Submitting author: @ashleychontos (Ashley Chontos)
Repository: https://github.com/ashleychontos/pySYD
Branch with paper.md (empty if default branch): master
Version: v6.10.0
Editor: @mbobra
Reviewers: @danhey, @benjaminpope
Archive: 10.5281/zenodo.7301604
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@danhey & @benjaminpope, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @mbobra know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Review checklist for @danhey
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @benjaminpope
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: