search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
707 stars 37 forks source link

[REVIEW]: TransBigData: A Python package for transportation spatio-temporal big data processing, analysis and visualization #4021

Closed whedon closed 2 years ago

whedon commented 2 years ago

Submitting author: !--author-handle-->@ni1o1<!--end-author-handle-- (Qing Yu) Repository: https://github.com/ni1o1/transbigdata Branch with paper.md (empty if default branch): Version: v0.3.7 Editor: !--editor-->@martinfleis<!--end-editor-- Reviewers: @jGaboardi, @anitagraser Archive: 10.5281/zenodo.6236507

:warning: JOSS reduced service mode :warning:

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

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/d1055fe3105dfa2dcff4cb6c7688a79b"><img src="https://joss.theoj.org/papers/d1055fe3105dfa2dcff4cb6c7688a79b/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/d1055fe3105dfa2dcff4cb6c7688a79b/status.svg)](https://joss.theoj.org/papers/d1055fe3105dfa2dcff4cb6c7688a79b)

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

@jGaboardi & @anitagraser, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @martinfleis 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 @jGaboardi

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @anitagraser

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 2 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @jGaboardi, @anitagraser it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

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.

:star: Important :star:

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:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 2 years ago

Wordcount for paper.md is 630

whedon commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.jclepro.2020.119974 is OK
- 10.1016/j.jclepro.2020.122471 is OK
- 10.1016/j.trc.2020.102672 is OK
- 10.1016/j.apenergy.2020.115038 is OK
- 10.1016/j.jclepro.2020.125567 is OK
- 10.1177/03611981211036684 is OK
- 10.1016/j.ijtst.2021.01.002 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 2 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.37 s (175.7 files/s, 69976.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
XML                              1              0              0           9484
reStructuredText                21           1332           2763           1887
Python                          17            194           1059           1875
Markdown                         3            113              0            484
CSS                              1             91             11            405
Jupyter Notebook                 7              0           5352            405
TeX                              1              8              0            117
JSON                             5              0              0             85
YAML                             4             16             10             64
HTML                             2             16              0             63
DOS Batch                        1              8              1             26
make                             1              4              7              9
SVG                              1              0              0              3
-------------------------------------------------------------------------------
SUM:                            65           1782           9203          14907
-------------------------------------------------------------------------------

Statistical information for the repository 'f6e8f14381b53d89e9fddf9a' was
gathered on 2021/12/21.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
ni1o1                            5            72             48            3.33
yuanjian24                       5            79              3            2.27
余庆                            45          2983            422           94.40

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
ni1o1                        60           83.3          0.3                0.00
yuanjian24                   48           60.8          0.0                0.00
余庆                       3020          101.2          1.1                8.34
whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

martinfleis commented 2 years ago

👋🏼 @ni1o1 @anitagraser @jGaboardi this is the review thread for the paper. All of our communications will happen here from now on.

Both reviewers have checklists at the top of this thread with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#4021 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks but considering upcoming holidays, feel free to start whenever it works for you. Please let me know if any of you require significantly more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@martinfleis) if you have any questions/concerns.

martinfleis commented 2 years ago

Moving here a comment by @anitagraser from the pre-review (https://github.com/openjournals/joss-reviews/issues/3986#issuecomment-998799093)

I haven't had chance to look at the code yet. Concerning the paper, I think there is one point in the review criteria that is currently not addressed:

A description of how this software compares to other commonly-used packages in this research area.

whedon commented 2 years ago

:wave: @jGaboardi, please update us on how your review is going (this is an automated reminder).

whedon commented 2 years ago

:wave: @anitagraser, please update us on how your review is going (this is an automated reminder).

jGaboardi commented 2 years ago

👋 @jGaboardi, please update us on how your review is going (this is an automated reminder).

This is still very much on my radar, but several personal issues have cropped in the last week. I am going to try to get back to it within the next 2 weeks or so.

anitagraser commented 2 years ago

👋 @anitagraser, please update us on how your review is going (this is an automated reminder).

Same here. Will get back to it soon.

jGaboardi commented 2 years ago

@ni1o1 @anitagraser @martinfleis

I have completed my first thorough pass of transbigdata and the manuscript, and have opened several issues and a PR, mostly involving documentation. Once the minor language issues, etc. are fixed in the paper, I would comfortable checking off those boxes. However, a sticking point that will ultimately hold back JOSS acceptance is the apparent lack of any testing being performed (ni1o1/transbigdata#7). This must be addressed.

jGaboardi commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

PDF failed to compile for issue #4021 with the following error: 

 /app/vendor/bundle/ruby/2.6.0/gems/octokit-4.8.0/lib/octokit/response/raise_error.rb:16:in `on_complete': GET https://api.github.com/repos/JuliaCon/proceedings-review/issues/4021: 404 - Not Found // See: https://docs.github.com/rest/reference/issues#get-an-issue (Octokit::NotFound)
    from /app/vendor/bundle/ruby/2.6.0/gems/faraday-0.15.4/lib/faraday/response.rb:9:in `block in call'
    from /app/vendor/bundle/ruby/2.6.0/gems/faraday-0.15.4/lib/faraday/response.rb:61:in `on_complete'
    from /app/vendor/bundle/ruby/2.6.0/gems/faraday-0.15.4/lib/faraday/response.rb:8:in `call'
    from /app/vendor/bundle/ruby/2.6.0/gems/octokit-4.8.0/lib/octokit/middleware/follow_redirects.rb:73:in `perform_with_redirection'
    from /app/vendor/bundle/ruby/2.6.0/gems/octokit-4.8.0/lib/octokit/middleware/follow_redirects.rb:61:in `call'
    from /app/vendor/bundle/ruby/2.6.0/gems/faraday-0.15.4/lib/faraday/rack_builder.rb:143:in `build_response'
    from /app/vendor/bundle/ruby/2.6.0/gems/faraday-0.15.4/lib/faraday/connection.rb:387:in `run_request'
    from /app/vendor/bundle/ruby/2.6.0/gems/faraday-0.15.4/lib/faraday/connection.rb:138:in `get'
    from /app/vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/agent.rb:94:in `call'
    from /app/vendor/bundle/ruby/2.6.0/gems/octokit-4.8.0/lib/octokit/connection.rb:156:in `request'
    from /app/vendor/bundle/ruby/2.6.0/gems/octokit-4.8.0/lib/octokit/connection.rb:19:in `get'
    from /app/vendor/bundle/ruby/2.6.0/gems/octokit-4.8.0/lib/octokit/client/issues.rb:114:in `issue'
    from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-c5c16aedb3d6/lib/whedon/review.rb:21:in `issue_body'
    from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-c5c16aedb3d6/bin/whedon:44:in `prepare'
    from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/command.rb:27:in `run'
    from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in `invoke_command'
    from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor.rb:387:in `dispatch'
    from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/base.rb:466:in `start'
    from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-c5c16aedb3d6/bin/whedon:131:in `<top (required)>'
    from /app/vendor/bundle/ruby/2.6.0/bin/whedon:23:in `load'
    from /app/vendor/bundle/ruby/2.6.0/bin/whedon:23:in `<main>'
jGaboardi commented 2 years ago

This is a rather strange failure, no? Seems to be trying for a JuliaCon repo?

https://api.github.com/repos/JuliaCon/proceedings-review/issues/4021
martinfleis commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

ni1o1 commented 2 years ago

@ni1o1 @anitagraser @martinfleis

I have completed my first thorough pass of transbigdata and the manuscript, and have opened several issues and a PR, mostly involving documentation. Once the minor language issues, etc. are fixed in the paper, I would comfortable checking off those boxes. However, a sticking point that will ultimately hold back JOSS acceptance is the apparent lack of any testing being performed (ni1o1/transbigdata#7). This must be addressed.

Thank you for your effort! We have fixed the testing issue by adding github testing workflow. We are working on other issues.

jGaboardi commented 2 years ago

@ni1o1 @anitagraser @martinfleis I have completed my first thorough pass of transbigdata and the manuscript, and have opened several issues and a PR, mostly involving documentation. Once the minor language issues, etc. are fixed in the paper, I would comfortable checking off those boxes. However, a sticking point that will ultimately hold back JOSS acceptance is the apparent lack of any testing being performed (ni1o1/transbigdata#7). This must be addressed.

Thank you for your effort! We have fixed the testing issue by adding github testing workflow. We are working on other issues.

Excellent. And very interesting package, by the way!

jGaboardi commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

anitagraser commented 2 years ago

We have fixed the testing issue by adding github testing workflow. We are working on other issues.

As far as I can see from https://github.com/ni1o1/transbigdata/runs/4740487080?check_suite_focus=true there is only one (docstring) test. Also all linting errors are ignored.

anitagraser commented 2 years ago

What is the JOSS policy regarding other languages than English?

For example, this library still has Chinese in error messages: image

I imagine this would violate the documentation requirement "Functionality documentation: Is the core functionality of the software documented to a satisfactory level".

jGaboardi commented 2 years ago

@anitagraser I noticed this, as well. I was not aware that JOSS had such a policy. Also there is an English language version of the docs. However, having all error messages in English does seem the way to go and would greatly benefit transbigdata.

anitagraser commented 2 years ago

@ni1o1 @martinfleis @jGaboardi

I have also finished my first pass through the paper, repo, and example notebooks. I've created new issues and added comments to @jGaboardi's issues where I noticed additional aspects. From my perspective, the lack of tests is the main issue with the repo. The lack of state of the art is the main issue with the paper.

martinfleis commented 2 years ago

Thanks both, @anitagraser and @jGaboardi!

@ni1o1 I am also a bit concerned about the fact that docstrings, error messages and comments are in Chinese and would like to see it in English. However, I am trying to figure out whether it is an issue hindering publication with the wider editorial team. I'll get back to you on that.

Also, please let us know how you plan to respond to the comments raised by reviewers and what is the expected timeline for that. Thanks!

ni1o1 commented 2 years ago

Thanks both, @anitagraser and @jGaboardi!

@ni1o1 I am also a bit concerned about the fact that docstrings, error messages and comments are in Chinese and would like to see it in English. However, I am trying to figure out whether it is an issue hindering publication with the wider editorial team. I'll get back to you on that.

Also, please let us know how you plan to respond to the comments raised by reviewers and what is the expected timeline for that. Thanks!

@martinfleis @anitagraser @jGaboardi Thank you all for reviewing our package! Here is how we plan to improve our package according to the comments:

  1. Translate all docstrings, error messages and comments into English.
  2. Add necessary tests into the repo.
  3. Add state of the art and state of the target audience into the paper.
  4. Other improvment of the docs and examples. We plan to finish them within three weeks.
martinfleis commented 2 years ago

Brilliant! Thanks!

martinfleis commented 2 years ago

@ni1o1 I am also a bit concerned about the fact that docstrings, error messages and comments are in Chinese and would like to see it in English. However, I am trying to figure out whether it is an issue hindering publication with the wider editorial team. I'll get back to you on that.

Just for the sake of completeness, the agreed editorial position on this is that the content (docstrings, comments, error messages) is subject to review and thus needs to be in the same language as the paper.

ni1o1 commented 2 years ago

OK, we will translate all that into English

ni1o1 commented 2 years ago

Hi @martinfleis @anitagraser @jGaboardi. Thank you for all the comments. We have finished our revision of the paper and the repo, details include:

  1. Language Translation: We have translated all docstrings, error messages and comments into English, include the Ipynb notebook examples.
  2. Adding Code Tests: Tests were added and the test coverage is now at 94%.
  3. Paper Enhancing: We added two parts into the paper:
    • The state of the art part introducing similar software in spatial-temporal data analysis field.
    • The statement of target audience in the state of need part.
  4. Other Improvements: We have added the contribution guidelines, code of conduct into the repo.
ni1o1 commented 2 years ago

@whedon generate pdf

ni1o1 commented 2 years ago

@whedon check references

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

whedon commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.compenvurbsys.2017.05.004 is OK
- 10.1108/9780857245670-016 is OK
- 10.1016/j.jclepro.2020.119974 is OK
- 10.1016/j.jclepro.2020.122471 is OK
- 10.1016/j.trc.2020.102672 is OK
- 10.1016/j.apenergy.2020.115038 is OK
- 10.1016/j.jclepro.2020.125567 is OK
- 10.1177/03611981211036684 is OK
- 10.1016/j.ijtst.2021.01.002 is OK
- doi.org/10.1111/2041-210X.13374 is OK
- 10.21105/joss.03607 is OK
- 10.1553/giscience2019_01_s54 is OK
- 10.21105/joss.01518 is OK
- 10.1007/978-3-642-03647-7_11 is OK
- 10.5281/zenodo.5528110 is OK

MISSING DOIs

- 10.1016/c2020-0-02866-5 may be a valid DOI for title: Big Data and Mobility as a Service

INVALID DOIs

- 2108.13202 is INVALID
ni1o1 commented 2 years ago

@whedon check references

whedon commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.compenvurbsys.2017.05.004 is OK
- 10.1108/9780857245670-016 is OK
- 10.1016/j.jclepro.2020.119974 is OK
- 10.1016/j.jclepro.2020.122471 is OK
- 10.1016/c2020-0-02866-5 is OK
- 10.1016/j.trc.2020.102672 is OK
- 10.1016/j.apenergy.2020.115038 is OK
- 10.1016/j.jclepro.2020.125567 is OK
- 10.1177/03611981211036684 is OK
- 10.1016/j.ijtst.2021.01.002 is OK
- doi.org/10.1111/2041-210X.13374 is OK
- 10.21105/joss.03607 is OK
- 10.1553/giscience2019_01_s54 is OK
- 10.21105/joss.01518 is OK
- 10.1007/978-3-642-03647-7_11 is OK
- 10.5281/zenodo.5528110 is OK

MISSING DOIs

- None

INVALID DOIs

- None
jGaboardi commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

jGaboardi commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

jGaboardi commented 2 years ago

@martinfleis

I have completed my second pass. Once ni1o1/transbigdata#17 & ni1o1/transbigdata#18 are merged and ni1o1/transbigdata#14 is addressed (fix PR in ni1o1/transbigdata#19), I believe all my comments will be resolved. That still leaves ni1o1/transbigdata#11 and ni1o1/transbigdata#12 that @anitagraser raised and anything else she may find.

@ni1o1, nice job responding to and addressing my concerns. Great work. Also, nice logo for the package.

jGaboardi commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

anitagraser commented 2 years ago

@martinfleis: I have completed my second pass. In this context, I tested the conda install and re-run the notebooks which revealed a few more issues that should be addressed.

martinfleis commented 2 years ago

Thank you @anitagraser and @jGaboardi!

@ni1o1 Can you let us know once you dealt with all the raised issues? Thanks!

jGaboardi commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

ni1o1 commented 2 years ago

Thank you @anitagraser and @jGaboardi!

@ni1o1 Can you let us know once you dealt with all the raised issues? Thanks!

@martinfleis @jGaboardi @anitagraser. Thank you for all the comments and efforts. We have finished our revision of the paper and the repo, all raised issues are solved.

martinfleis commented 2 years ago

Hi @anitagraser,

I think that the once you are happy we can close this review. Let me know if you have any further comments, I see that you still have some points above unchecked.