[REVIEW]: Halotools: A New Release Adding Intrinsic Alignments to Halo Based Methods #7421
Comments
editorialbot
added
Cython
Python
review
TeX
Track: 1 (AASS)
|
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. 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: |
|
|
Software report: Commit count by author: |
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
|
License info: 🔴 Failed to discover a valid open source license |
|
@matroxel, @cmlamman welcome to the review issue! We are looking for a review in 3-4 weeks. It would be great if we can aim for the end of November. For instructions on generating the review checklist, see the top of this issue. This review is for contributions to "The short version is that all commits made by me @.***) are part of If you have any questions, don't hesitate to tag me here or send me an email. |
|
@editorialbot add @fjcastander as reviewer |
|
@fjcastander added to the reviewers list! |
@@nvanalfen, I see that the license is basically a BSD 3-clause, but done in a non-standard way. Do you think it would be possible to refactor it to a standard BSD 3-clause? Also tagging @aphearin. |
|
Happy to refactor @ivastar - any templates I should look at? |
astropy/halotools#1105 should do it. And then @nvanalfen can pull to his fork to bring the changes over. |
|
@ivastar I have synced my fork to include these changes. Thank you both of you! |
Review checklist for @cmlammanConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
|
@nvanalfen I'm having a problem with reproducing the IA tutorials: nvanalfen/halotools#9 (comment) |
|
Thanks for your patience @nvanalfen on the above issue - I will reinstall everything in a fresh environment as soon as I get a chance. |
Yes, that's fine. As long as we are making progress. |
|
@matroxel, pinging you about this review. Please let me know if you are encountering issues. @fjcastander, I added you as an optional reviewer. You are welcome to generate the review list as instructed at the top and only review parts that align with your expertise. |
Review checklist for @matroxelConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
|
I'm happy to approve - I suggested a very minor documentation fix in the open issue related to required python version for the environment, which should be addressed. I did not run into the issues @cmlamman did with running the suggested examples. |
|
Thank you for the review @matroxel! |
|
@ivastar I've completed my review! |
|
@cmlamman thank you! Can you please formally accept for publication? |
|
@editorialbot generate pdf |
|
@editorialbot check references |
|
@editorialbot check repository |
|
|
Software report: Commit count by author: |
|
Paper file info:
✅ The paper includes a |
|
License info: ✅ License found: |
|
@nvanalfen Below is a round of editorial notes, please implement if possible. Also, please trim your BIB file to only the references used in the paper, our checker checks all references, not just the ones used, which results also in the flagging of the references without DOIs (which are actually not used). In the remaining references, please replace shortcuts such as \apj and \aj with the full name of the journal. If there are any arXiv references, please check if you can replace them with proper journal references. Let me know if you have any questions. Let me know when that's complete and I'll move to the post-review tasks. Line 10: initially release --> originally published |
|
@ivastar Sorry for that and thank you for the clear list. Here's my summary of changes:
For the one change I haven't (yet) made:
I have made the first change ("as such" -> "therefore"), but for the second (assuming I'm looking at the right spot), the current sentence is:
The suggested change only altered "modeling this" to "modeling of this". I wasn't sure if you saw "accurately modeling this" as "accurate modeling this", so I have left it as is for now. If you feel that the full change Thank you so much for all your time and hard work. |
|
@editorialbot generate pdf |
|
@editorialbot check references |
|
No strong feelings on that one, maybe I did read it as "accurate modeling", it's fine to leave as is. |
|
Doh, forgot one comment. Can you redo the Figure 1 caption to something like "Correlation functions from IllustrisTNG300.... showcasing the flexibility of the model. Reproduced from Figure 12 in Van Alfen et al. (2024)." |
Post-Review Checklist for Editor and AuthorsAdditional Author Tasks After Review is Complete
Editor Tasks Prior to Acceptance
|
|
@nvanalfen we can proceed to the post-review checklist. Please follow the list above. For the release, remind me what were we going to do here? Tag a release of |
|
@ivastar Andrew has tagged and released the IA version of halotools as v0.9.0 (it looks like minor tweaks since then have advanced to the current version being v0.9.2, but v0.9.0 is where this work was merged in). Should I formally mark that somewhere here? I have also fixed and pushed the caption 1 edit. Thank you for catching that. Also, looking at the generated PDF and I see that my author affiliation list is less of a list and more of a paragraph. It seems I haven't mastered the art of formatting that correctly. I have included them in the markdown as follows. Should I alter that?
|
|
@aphearin I'm unfamiliar with the release and archive process, so I'm tagging you here in case you know the answers to what I'm about to ask. It looks like the latest version (v0.9.2) is where we included the license change. @ivastar , this seems to be the latest change we made to the code. Is this sufficient? If so, does the PyPI entry for Halotools v0.9.2 count as the archive? I can't seem to find the DOI on that page. If not, I can (with Andrew's help potentially since I don't want to go archiving a package I'm not the maintainer for) go ahead and archive that on something more appropriate. Apologies for all the questions and thank you for all your help. |
|
@editorialbot generate pdf |
This is the typical formatting. I don't see anything wrong with it. It's ok to leave as is. |
The PyPI release does not count as archive. It should be possible to archive this version to Zenodo, seems like there are 2 releases already: https://zenodo.org/records/835895 |
|
@nvanalfen @aphearin quoting from the acting editor in chief: "The principle behind the archive, I believe, is to have a permanent archive of the code that was reviewed for the paper, which to me implies that we should archive the fork, unless the reviewed code is merged into the upstream According to the pre-review discussion, the changes here have already been merged upstrea. So, does any of the existing Zenodo archives have these changes? In that case, we can point to that DOI. Or can you make a new Zenodo archive of |
|
All of the code changes have been merged into the official @ivastar Two questions:
|
Yes, that's ok in this case as long as all authors of the paper are included as archive authors.
Also, ok to have a mismatch in this case. |
|
@ivastar Sorry for the delay. Just and update: Andrew and I are working on getting access to the existing Zenodo archive for halotools so I can upload a new version. Once I manage that, I will post the DOI and other required information here. Thank you for your patience and help. |
|
@nvanalfen any luck with Zenodo? |
|
@ivastar not yet. I'll ping the channel again where we're talking about it. Sorry for the delay. |
Submitting author: @nvanalfen (Nicholas Van Alfen)
Repository: https://github.com/nvanalfen/halotools
Branch with paper.md (empty if default branch): paper
Version: v0.9
Editor: @ivastar
Reviewers: @matroxel, @cmlamman, @fjcastander
Archive: Pending
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
@matroxel & @cmlamman, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @ivastar 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 ✨
Checklists
📝 Checklist for @cmlamman
📝 Checklist for @matroxel
The text was updated successfully, but these errors were encountered: