Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign up[REVIEW]: webweb: a tool for creating lightweight network visualizations in the browser #1458
Comments
whedon
assigned
cMadan
May 19, 2019
whedon
added
the
review
label
May 19, 2019
whedon
referenced this issue
May 19, 2019
Closed
[PRE REVIEW]: webweb: a tool for creating lightweight network visualizations in the browser #1308
This comment has been minimized.
This comment has been minimized.
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @vc1492a, it looks like you're currently assigned as the reviewer for this paper 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:
|
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
whedon
assigned
jg-you and
vc1492a
May 20, 2019
This comment has been minimized.
This comment has been minimized.
Overview: Overall, I think the author(s) did a good job on the software and in providing examples and documentation in order to get started easily. The software is easy to use and I think addresses the needs of the audience as well. While I would have liked to see more refinement in the code base prior to consideration for publication in JOSS (e.g. matching and clear format for version numbers, no unused parameters, static methods when needed, etc.), these items can easily be addressed in newer versions of the software prior to acceptance in JOSS. I particularly like the software's flexibility in the input data - being able to use lists, adjacency matrices, or networkx objects is a nice capability and opens up the software to a broader audience versus implementing functionality with networkx alone. Version Matching: The version number submitted to JOSS (0.0.29) does not match the latest version of the source code on the master branch in Github (0.0.30 as indicated in setup.py) - the JOSS paper and submission will need to be updated for the latest version of the software. Additionally, the version number does not match the version listed on PyPi (0.0.32 and the section titled Installation Instructions: The Github repository as well as the linked documentation provide instructions for installation via PyPi or by cloning the Github repository. After examining both Functionality: As indicated in the pre-review, I am only able to review this software's Python language functionality, specifically Python 3. The functionality works as described by the authors and I particularly like the functionality to view different layers of a network. I did notice a small detail that perhaps could be addressed in future versions of the software. When viewing multiple networks as described in the documentation examples, the viewing options (e.g. node color, size, etc.) are reset when transitioning from one network to another. It would be nice if the previous viewing options were saved in a from of cache so that users do not need to reconstruct a network's view when transitioning from one network to another. This isn't a big deal when using toy examples, but I can imagine some users may be frustrated if constructing a particular view in network A, transitioning to network B, only to find that the view is gone when transitioning back to network A. The ability to zoom into and out of a visualized network using the mouse or trackpad would be a welcome addition as well. Lastly, while the I noticed a lot of use of Python's Lastly, the GUI in webweb seems to share a lot of similar capabilities and features with the GUI used in netwulf. Although the aims of each software are different, both sets of authors could potentially reduce future development effort by collaborating on shared features and/or components. Automated Tests: While I did see a Example Usage: The software documentation does include some examples on how to use the software, which I found helpful for reviewing the software's functionality. I also saw a section for real-world examples which is great! References: Community Guidelines: The Github repository as well as the documentation contains some guidelines on how to report issues and how to contribute to webweb's development which is great! It may benefit the authors / developers however to include more specific contribution instructions, such as asking users to first fork the repository and then create a new branch with a name that describes the issue or new feature before submitting a pull request. This may result in some additional organization. Additionally, while there is a section titled Other: I made a series of other small updates which are contained in the aforementioned pull request, such as moving imports to the top of the files, converting functions that don't use object attributes and functions to static methods, adding numpy to |
This comment has been minimized.
This comment has been minimized.
@cMadan I have provided a view of |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hneutr
commented
Jun 5, 2019
I've updated I apologize if I'm missing where this is described, but I'm not sure how to update the JOSS submission.
I agree this would be useful. I'm unfamiliar with the tagging process, but am reading up now, and have created an issue to represent this.
Done. I've updated the readme to point out the requirement of numpy, as well as clarified the index page of the documentation site to make clear that the visualizations have no dependencies, as opposed to the software used to generate those visualizations. Great point!
I absolutely agree. I think this pretty annoying. I've added a feature request to represent this, and quoted you as I think you convey the issue and solution well.
There's an issue for this — I agree that it would be really useful!
No strong feelings to putting it above as opposed to below, but also not entirely sure how big a difference it would make. My general feeling is "if it ain't broke, don't fix it", especially with UI, but I'm certainly open to being convinced of otherwise. What draws you wanting it above?
Added an explicit disclaimer that the code may not function in python 2.
I've talked to Dan, and I think collaborating with them is a great idea, and will be reaching out.
I've added a bunch of automated tests that
I agree that this would be desirable, and have opened an issue to represent this.
I've merged the pull request. Thank you for taking the time to add the DOIs!
@cMadan, is this something we can do?
Done!
I added a
Merged! Thank you again for taking the time to do this.
Added! |
This comment has been minimized.
This comment has been minimized.
hneutr
commented
Jun 5, 2019
@vc1492a thank you for your thorough review! I believe I've addressed your points — they were all excellent, and improved the software. The perspective of a seasoned python developer is much appreciated! @cMadan I've responded to @vc1492a's review above! I believe I hit everything mentioned, but if there's any points I've missed, please let me know and I'll circle back. |
This comment has been minimized.
This comment has been minimized.
@hneutr Thanks for your detailed response! I've examined the updates and they look good to me. In reply to your question about what drives me to prefer that ordering on the UI, it just seems to be that a user would first toggle on the option to show node names before doing a secondary filtering. |
This comment has been minimized.
This comment has been minimized.
hneutr
commented
Jun 7, 2019
@vc1492a I buy your logic, and have swapped the widget positions! |
This comment has been minimized.
This comment has been minimized.
jg-you
commented
Jun 11, 2019
I have been using 0.0.18 in my work for a while, and updated to version 0.0.35, the latest release on PyPI, for this review. Overall, the package passes almost all the checks with very few exceptions. Version numbers: I'll second @vc1492a in saying that tagging releases on github would be nice, for a different reason: Documentation: The examples listed on the website are fantastic for getting an intuitive grasp of what the package is doing. Installation: The numpy dependency now appears everywhere on github What is below is not part of my review per se, but just little comments. Possible features?: Some things that would be great to have at some point.
Minor things: Consider maintaining the author information in |
This comment has been minimized.
This comment has been minimized.
jg-you
commented
Jun 11, 2019
@cMadan My review is above! Thanks for your patience everyone. |
This comment has been minimized.
This comment has been minimized.
hneutr
commented
Jun 18, 2019
•
Hey @jg-you! Thank you for the thorough review, and the great suggestions. Responses inline below:
Upping this priority.
done in
Added.
Made an issue.
I think this is an awesome idea, very much inline with the "spirit" of
Made an issue.
done in
done in |
This comment has been minimized.
This comment has been minimized.
jg-you
commented
Jun 21, 2019
This comment has been minimized.
This comment has been minimized.
@vc1492a @jg-you, thank you both for your thorough reviews, I greatly appreciate it! @hneutr, it looks like you're still not using GitHub releases? (And I don't see a response to that suggestion.) The netwulf DOI will be 10.21105/joss.01425. I am fine with you citing it here, with this DOI (even though not officially registered yet) as it will resolve as a citation to the other package once it is published. Otherwise I think the review is nearly done and I just need to do some final checks. |
This comment has been minimized.
This comment has been minimized.
hneutr
commented
Jul 8, 2019
I've added the netwulf DOI to the Let me know if there is anything else that needs to be done! |
This comment has been minimized.
This comment has been minimized.
@hneutr, everything looks good to me! To move forward with accepting your submission, there are a few last things to take care of:
You may find this helpful: https://guides.github.com/activities/citable-code/ |
This comment has been minimized.
This comment has been minimized.
hneutr
commented
Aug 12, 2019
This comment has been minimized.
This comment has been minimized.
@whedon set 0.0.37 as version |
This comment has been minimized.
This comment has been minimized.
OK. 0.0.37 is the version. |
This comment has been minimized.
This comment has been minimized.
@whedon set 10.5281/zenodo.3366140 as archive |
This comment has been minimized.
This comment has been minimized.
OK. 10.5281/zenodo.3366140 is the archive. |
This comment has been minimized.
This comment has been minimized.
@hneutr, great! I think you're all good! @openjournals/joss-eics, we're all set to accept here! |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
@whedon accept |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
Check final proof If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#898, then you can now move forward with accepting the submission by compiling again with the flag
|
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
there's a bib typo, fixed in dblarremore/webweb#53 |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
|
This comment has been minimized.
This comment has been minimized.
dblarremore
commented
Aug 12, 2019
@danielskatz I just merged it. Good catch. |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
@whedon accept |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
Check final proof If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#899, then you can now move forward with accepting the submission by compiling again with the flag
|
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Aug 12, 2019
@whedon accept deposit=true |
This comment has been minimized.
This comment has been minimized.
|
whedon
added
the
accepted
label
Aug 12, 2019
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
Here's what you must now do:
Any issues? notify your editorial technical team... |
danielskatz
closed this
Aug 12, 2019
This comment has been minimized.
This comment has been minimized.
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! 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:
|
whedon commentedMay 19, 2019
•
edited
Submitting author: @hneutr (Kenneth Hunter Wapman)
Repository: https://github.com/dblarremore/webweb
Version: 0.0.37
Editor: @cMadan
Reviewer: @vc1492a, @jg-you
Archive: 10.5281/zenodo.3366140
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
@vc1492a & @jg-you, 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 @cMadan know.
Review checklist for @vc1492a
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @jg-you
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?