-
-
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]: pref_voting: The Preferential Voting Tools package for Python #7020
Comments
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: ✅ License found: |
Hello again! 👋
This is the review thread for the paper. All of our higher-level communications will happen here from now on, review comments and discussion can happen in the repository of the project (details below). 📓 Please read the "Reviewer instructions & questions" in the comment from our editorialbot (above). |
FYI: I will be out of office for 2.5 weeks. 🌲 🌻 I will check-in again as soon as I am back! In the meanwhile, see above for how to get your reviewing process started! 🌱 |
Review checklist for @dmnapolitanoConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Thanks for getting started with your review, @dmnapolitano ! |
For transparency: I just sent @pkrafft and email in case the Github notifications are off 🙂 |
Hi @britta-wstnr , I believe I'm done with my review 🎉 Sorry it took so long, and please let me know if there's anything I missed or anything else I can help out with here. One question I have...it's minor but the authors use "etc." quite a bit throughout the paper, which is rather informal compared to the rest of the paper (which is very well-written!). Is this ok for JOSS? This is my first time reviewing here so I'm not entirely familiar with the style standards 🙂 Thanks!! |
Review checklist for @pkrafftConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hi @dmnapolitano - thanks a lot for your review! 🙏 Regarding your question on style, AFAIK we do not have a strict style guide on this, but if something is disturbing the reading flow, it's definitely worth changing. I had a look at the use of etc. in the paper, and it might indeed make the paper a little more elegant to read if some of those occurrences could be resolved differently in the text. @epacuit can I add this onto your list? Thanks 🙏 and thanks to @dmnapolitano for pointing it out. |
Hi @britta-wstnr , thanks for following up! Yes, I'm happy with how the issue was handled and I hope it'll be clarified in the documentation soon if it wasn't already. My issue will hopefully help others, at the very least 😄 And thanks, I agree that "etc." feels out of place in this otherwise well-written paper! 🙌🏻 |
@britta-wstnr @dmnapolitano Thanks for the suggestion about removing "etc.". We have updated the paper accordingly. |
Hi all and @britta-wstnr, for minor issues, should I make a list in an issue comment, or make direct edits/suggestions through a fork and pull requests? |
Hi @pkrafft - both works, whatever you find more convenient. If it's a list of minor comments, you can even post that in the thread here if you prefer. Bigger things we think is better documented at the software-level, i.e., on the Github repo of the software (that makes it easier for people to find it back in the future). Then just make sure to link the issue or PR here. |
@pkrafft gentle ping just to make sure this answered your question? 🙂 |
The issues I've identified in my review are as follows. I think the package, documentation, and write-up are okay but with room for improvement. They are certainly above the standard I would expect for research code but don't quite meet a high open source/free software standard.
|
Thank you for the feedback! We are still in the process of updating the README along the lines of your suggestions. Regarding the errors with the tests and the import issues:
These are both fixed in the commits 8856e72c890d8d7d2b02e856fac499d13c91cd96 and 95f40df3841d1f19419cfc7e23af22553348674f |
👋 @openjournals/sbcs-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#6352, then you can now move forward with accepting the submission by compiling again with the command |
@britta-wstnr thanks so much to you and @pkrafft ! This was my first time reviewing for JOSS and I learned a lot about the process from both of you 😄 🎉 👏🏻 |
Thanks everyone for your work on this! |
Sorry, but there is one thing that we just noticed, there is a bib entry that has an incorrect date: Holliday, W. H., Kristoffersen, A., & Pacuit, E. (ForthcomingForthcoming). Learning to Do you know how to make sure it says "Forthcoming" rather than "ForthcomingForthcoming". The entry in the bib file is correct: @inproceedings{HKP2025, Usually this works fine, but is there a requirement that "year" field is a number? |
Mhh ... good question - I do not know the answer. Let's see if the EiC knows once they get to this, otherwise I will inquire with the rest of the team! |
I think that there is a bug when processing the bib file. If the year field is a string, then it will write the field twice. To test, this I set the year field to the string "2025" (i.e., I used year={"2025"}), and this is what is produced: Holliday, W. H., Kristoffersen, A., & Pacuit, E. (”2025””2025”). Learning to manipulate We can fix this by setting the year to 2025 (the paper is coming out at the next AAAI meeting in a little over a month...). I've done that now, so can we use the most recent version in our repo. Thanks!! |
Whew, I can't see immediately why it does that. If it's an ongoing issue we can tag in the dev team, but sounds like it's not an issue for now! |
I just wanted to check if there is anything else I need to do to ensure that the issue with the bib file is corrected in the final version of the paper? The latest version in our repo removed the "Forthcoming" and replaced with with an integer so to bypass the bug with processing the bib file. Thanks! |
@editorialbot generate pdf |
@editorialbot check references |
|
@samhforbes do I have to |
That's OK I can do that now. |
@editorialbot recommend-accept |
|
|
👋 @openjournals/sbcs-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#6383, then you can now move forward with accepting the submission by compiling again with the command |
@editorialbot accept |
|
Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository. If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file. You can copy the contents for your CITATION.cff file here: CITATION.cff
If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation. |
🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘 |
🦋🦋🦋 👉 Bluesky post 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... |
Congrats @epacuit on the paper! Many thanks to @dmnapolitano and @pkrafft for reviewing, and of course @britta-wstnr for editing this one. |
🎉🎉🎉 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: @epacuit (Eric Pacuit)
Repository: https://github.com/voting-tools/pref_voting
Branch with paper.md (empty if default branch):
Version: 1.15.0
Editor: @britta-wstnr
Reviewers: @dmnapolitano, @pkrafft
Archive: 10.5281/zenodo.14675584
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
@dmnapolitano & @pkrafft, 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 @britta-wstnr 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 @dmnapolitano
📝 Checklist for @pkrafft
The text was updated successfully, but these errors were encountered: