-
-
Notifications
You must be signed in to change notification settings - Fork 38
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]: Netwulf: Interactive visualization of networks in Python #1425
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @camillescott, it looks like you're currently assigned as the reviewer for this paper 🎉. ⭐ 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:
|
|
@vc1492a @camillescott thank you for volunteering your time for this review 🥇 This is the review issue where you can leave comments and check items from your checklists. If you open any issues or PRs in the software repository, please remember to link back to this issue using @ulfaslak @vc1492a @camillescott please don't hesitate to reach to me if you have questions/concerns (here or privately, if you prefer). |
Overview: This was an interesting piece of software to review and I enjoyed walking through it and testing its functionality. While I believe updates are needed in order for publication in JOSS (see below), the software is well documented and seems to fit a niche that may be useful within the network analysis community. License: While a license is provided in the project repository (MIT), it may be beneficial to indicate the license more publicly in the readme on Github and/or in the documentation. For example, a badge could be used or a section added to the readme or documentation. Version Matching: The version number submitted to JOSS does match the latest version of the source code in master on Github. It may be beneficial however to tag the releases on Github that are uploaded to PyPi such that users can download the software directly from Github via a Release as opposed to having to clone or pull the repository. This is helpful to users in some cases and also provides the developer with a copy of the source code for previous versions. Functionality: I was able to verify the basic functionality, in this case installing the software, importing the software, and opening the visualization interface successfully by calling I'm not sure, but I think there may be a CSS formatting or similar error in the user interface. See the screenshot below: The Some potentially useful functionality would be to allow a user to start the visualization interface without the need to pass a graph object into Additionally, I noticed a lot of use of Python's Performance claims: The paper states "The interactive visualization is implemented in JavaScript, relies on d3.js for computing layouts, and uses the HTML5-object canvas for rendering. This makes it, to our knowledge, the most performant tool for interactive network visualization in Python to date." This statement implies that the word Functionality documentation: This documentation is great and contains details on the API as well as cookbooks containing example code. I was able to use the example code easily to get started on verifying the functionality of the software. I noticed one small item that could be addressed in the examples. In the below screenshot, it appears that a necessary I believe this should include Additionally, in the example located in the filtering section, The rest of the documentation looks great and is quite helpful! To gain additional feedback and user adoption, it may be interesting to show some insights gained through using the interactive visualization interface on some real-world data (not necessary though). Statement of need (documentation): A statement of need exists in the documentation, but the focus differs from that highlighted in the generated paper. The documentation states "Netwulf is an interactive visualization tool for networkx Graph-objects, that allows you to produce beautiful looking network visualizations. Simply input a networkx.Graph object, style the network in the interactive console and either download the result as a PNG or pipe the layout back to Python for further processing." This implies the software's focus is on creating visually-pleasing network visualizations easily from networkx objects. However, the paper states "Netwulf is a light-weight Python library that provides a simple API for interactively visualizing a network and returning the computed layout and style. It is build around the philosophy that network manipulation and preprocessing should be done programmatically, but that the efficient generation of a visually appealing network is best done interactively, without code. Therefore, it offers no analysis functionality and only few exploration features, but instead focuses almost entirely on fast and intuitive layout manipulation and node/link styling." This statement implies that the development focus is still in generating visually appealing networks interactively, but is mentions a programmatic interface and lack of analysis tools which obfuscates that meaning somewhat. The two statements are not in direct conflict, but the purpose of the work must be clearly stated and in agreement between the paper and the documentation and should be revised for clarity. Installation instructions: Instructions for installation work and are easy to follow, with the listed dependencies in Automated tests: I did not see any automated tests in this repository that could be used to verify the functionality of the software. Revisions of this work should include a series of unit tests that can be used to verify the functionality of the software, through testing of function outputs, input and output formatting, etc. There are a variety of ways to write and use unit tests in Python, two of which are the Community guidelines: While there are some development notes on how to update the front-end from it's upstream directory when developing, there are no clear guidelines on Github (in the readme or otherwise) or in the documentation on how to best jump into development or open a pull request for bug fixes or new features. Additionally, there is no documentation on how to report issues and it's implicitly implied that the user should open a Github issue. If that's the desired error reporting behavior, this should be made more explicit in the documentation. If Github issues is the desired mechanism, it may be beneficial to the developers to open issues for some of the behaviors noted here - for the authors: let me know if you'd like me to open issues for the items identified in the References: Other: Although the visuals current used are already nice and informative, they could also be improved in future versions. This would (potentially) be a nice place to collaborate with others with, and documentation on how to contribute to this and other areas would be nice to see in the repository and/or documentation. Also consider adding a |
@vc1492a thank you for the detailed review 🚀 @ulfaslak please do your best to respond to Valentino's comments either here in this thread or in separate issues in your repository (preferred). If you open the issues/PRs, please remember to link back to this thread by including |
I agree with @vc1492a regarding using
While manual tests are acceptable, use of some form of automated test is encouraged. |
@vc1492a, indeed thank you very much for the super detailed review and @leouieda thank you for the thorough editing process so far. Since @ulfaslak and me are both on vacation at the moment, we will be responding to the review in detail afterwards. In the mean time, @vc1492a would you be so kind as to open an issue for the |
We would like to thank @vc1492a again for his incredibly helpful review which sparked a lot of ideas which actually already improved the package. We were delighted to read that he found
We have added a
We introduced proper versioning and release tagging. @leouieda, we will probably change the version during the review process. Can we change the ultimate version number for JOSS once the software is accepted for publication?
This was discussed and resolved individually here: benmaier/netwulf#20 and here: benmaier/netwulf#21.
This was discussed and resolved individually here: benmaier/netwulf#23
This point seems to be up to the editor.
We fixed the bugs in the documentation code (see benmaier/netwulf#22). We think showing how
We agree and resolved this here: benmaier/netwulf#24
We added dependency statements in
We discussed this here: benmaier/netwulf#26. While completely automated tests seem to be complicated to implement due to both the the human component in the network styling process as well as the interplay between Python and JavaScript in a Browser window, we introduced automated tests which can be run by using the command
We've added a contributing guide and a code of conduct.
@vc1492a already added the DOI's for us. Thank you! :)
We indeed plan to improve the visualization further, e.g. by adding directed and arrowed links (see e.g. ulfaslak/network_styling_with_d3#3 and benmaier/curved-edges ). Furthermore I'd like to have additional styling functions for trees and hierarchically clustered networks. We created an OUTLOOK.md to keep track of these ideas. We hope that @vc1492a is content with our changes and are looking forward to @camillescott's review, too! |
@benmaier Thank you for the detailed replies to my comments. Thank you for writing some automated tests and for providing a structure for writing future tests. The issue of writing tests for an interactive framework is an interesting one, and I think worth exploring in the future as you continue to improve netwulf. I'd very much welcome seeing the addition of curved edges and clustered networks in netwulf as well! Great work. @leouieda At this stage, I believe we can consider my review of netwulf complete - please let me know if I can further assist in any way. |
👋 Hi @camillescott just checking in to see if you've made any progress on the review. Do you know when you'd be able to report back to us? |
Hi @camillescott just checking in again. Please let me know if you'll be able to complete the review. If, for any reason, you find that you will not be able to do this, just let me know and I can search for another reviewer. |
Hi @leouieda, just commenting to let you know that I've added a few features to the package because I needed them for my own research. We're now at v0.1.3. Is this going to be a problem? If it helps, everything @vc1492a reviewed is still functional in the same way he reviewed. I've also added tests and sections in the docs for the new features. |
|
@whedon check references |
|
|
Sorry for the spam folks! @leouieda, this looks fine to me now. |
👋 Hey @benmaier... Letting you know, |
@benmaier thank you for being on top of this! |
@openjournals/joss-eics this submission is ready of acceptance 🎊 |
Hi @benmaier, I fixed the NetworkX reference, please merge this PR: benmaier/netwulf#35 |
Done! |
@whedon generate pdf |
|
@whedon accept |
|
|
Check final proof 👉 openjournals/joss-papers#995 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#995, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🐦🐦🐦 👉 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... |
Congrats @ulfaslak on your article's publication in JOSS! Many thanks to @leouieda for editing, and @camillescott and @vc1492a for reviewing, this submission. |
🎉🎉🎉 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! 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: @ulfaslak (Ulf Aslak)
Repository: https://github.com/benmaier/netwulf
Version: 0.1.4
Editor: @leouieda
Reviewers: @camillescott, @vc1492a, @leouieda
Archive: 10.5281/zenodo.3402870
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
@camillescott & @vc1492a, 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 @leouieda know.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @camillescott
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 @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 @leouieda
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?The text was updated successfully, but these errors were encountered: