openjournals / joss-reviews

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

[REVIEW]: spopt: a python package for solving spatial optimization problems in PySAL #3330

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: !--author-handle-->@xf37<!--end-author-handle-- (Xin Feng) Repository: https://github.com/pysal/spopt Branch with paper.md (empty if default branch): Version: 0.4.1 Editor: !--editor-->@vissarion<!--end-editor-- Reviewers: @tmickleydoyle, @apulverizer Archive: 10.5281/zenodo.6638721

: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/1413cf2c0cf3c561386949f2e1208563"><img src="https://joss.theoj.org/papers/1413cf2c0cf3c561386949f2e1208563/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/1413cf2c0cf3c561386949f2e1208563/status.svg)](https://joss.theoj.org/papers/1413cf2c0cf3c561386949f2e1208563)

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

@apulverizer & @tmickleydoyle , 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 @vissarion 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 @apulverizer

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @tmickleydoyle

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ArikaLZ, @samuel-rosa 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 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.21 s (329.9 files/s, 125358.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                              2              0             15           8419
Python                          29           1066           2633           2811
Jupyter Notebook                13              0           9542            641
YAML                            11              7             14            319
Markdown                         3             55              0            165
TeX                              2             20              2            157
CSS                              1             11              3             64
reStructuredText                 5             67             89             58
DOS Batch                        1              8              1             27
make                             1              6              6             16
JSON                             1              0              0              1
-------------------------------------------------------------------------------
SUM:                            69           1240          12305          12678
-------------------------------------------------------------------------------

Statistical information for the repository '6983c40ec08e3a4710e59e13' was
gathered on 2021/06/02.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
James Gaboardi                  35          1217            408            3.60
Pedro Camargo                    1             2              2            0.01
Serge                            1             7             14            0.05
Serge Rey                       23          2663            296            6.56
Xin Feng                        27         21564            353           48.61
eli knaap                        9            39          17963           39.93
rwei5                            6           304            254            1.24

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
James Gaboardi              934           76.7         11.3                4.60
Pedro Camargo                 2          100.0          0.0                0.00
Serge Rey                  2140           80.4          6.4                9.58
Xin Feng                   3384           15.7          6.0                7.03
eli knaap                    18           46.2          4.3               27.78
rwei5                        32           10.5         22.3               18.75
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- 10.1080/13658810600665111 may be a valid DOI for title: Efficient Regionalization Techniques for Socio-economic Geographical Units Using Minimum Spanning Trees
- 10.2307/1907301 may be a valid DOI for title: Optimum utilization of the transportation system
- 10.1016/j.omega.2019.102176 may be a valid DOI for title: Contemporary optimization application through geographic information systems
- 10.1080/13658816.2020.1759806 may be a valid DOI for title: Efficient Regionalization for Spatially Explicit Neighborhood Delineation
- 10.2307/622300 may be a valid DOI for title: A Geographical Solution to Scale and Aggregation Problems in Region-building, Partitioning and Spatial Modelling
- 10.1068/a270425 may be a valid DOI for title: Algorithms for Reengineering 1991 Census Geography
- 10.1007/978-3-642-03647-7_11 may be a valid DOI for title: PySAL: A Python library of spatial analytical methods
- 10.3390/ijgi4020815 may be a valid DOI for title: Open geospatial analytics with PySAL
- 10.31219/osf.io/yzt2p may be a valid DOI for title: Spatially-encouraged spectral clustering: a technique for blending map typologies and regionalization

INVALID DOIs

- https://doi.org/10.1111/j.1467-9787.2011.00743.x is INVALID because of 'https://doi.org/' prefix
whedon commented 3 years ago

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

vissarion commented 3 years ago

Hi @xf37 it seems that several DOIs are missing from your submission and one is invalid as you can see above. Please fix this issue.

whedon commented 3 years ago

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

whedon commented 3 years ago

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

samuel-rosa commented 3 years ago

@whedon unfortunately I will not be able to conclude the review in due time. I have just been hit by a truckload of demands at my university. I hope that you find a suitable reviewer.

whedon commented 3 years ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands
vissarion commented 3 years ago

@whedon unfortunately I will not be able to conclude the review in due time. I have just been hit by a truckload of demands at my university. I hope that you find a suitable reviewer.

Hi @samuel-rosa that was an automated reminder from Joss robot (@whedon).

Just to be clear, if still interested in reviewing and you need more time to review this is totally understandable and we are very flexible especially during the codiv-19 era. On the other hand, if you decided that you cannot or do not want to review this is also understandable. Please let me know.

jGaboardi commented 3 years ago

@whedon generate pdf

whedon commented 3 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 3 years ago

Hi @xf37 it seems that several DOIs are missing from your submission and one is invalid as you can see above. Please fix this issue.

We have fixed the issue with missing DOIs.

vissarion commented 3 years ago

Hi, @nickbearman, @ryankarlos, @ystouthart I think this submission matches your interests.

Would any of you be willing to review this submission for JOSS?

We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html

vissarion commented 3 years ago

@whedon remove samuel-rosa as reviewer

whedon commented 3 years ago

OK, samuel-rosa is no longer a reviewer

vissarion commented 3 years ago

@whedon remove @samuel-rosa as reviewer

whedon commented 3 years ago

OK, @samuel-rosa is no longer a reviewer

nickbearman commented 3 years ago

Yes, happy to review. Just to check, I am not actually going to be able to run the package to test it out (not currently having the time/space to setup a Python environment), is this a major issue? If so, happy to pass the review to someone else. Very happy to complete the rest of the review elements.

vissarion commented 3 years ago

Hi @nickbearman thank you very much for you interest and quick response!

I am afraid it is important to test the functionality of the package as stated in https://joss.readthedocs.io/en/latest/review_criteria.html

Reviewers are expected to install the software they are reviewing and to verify the core functionality of the software.

nickbearman commented 3 years ago

@vissarion no problem. Sometime I will be able to setup a Python environment, but not for this one. Please do remove me as reviewer. Thanks.

samuel-rosa commented 3 years ago

@vissarion I really would like to be able to review this manuscript. In fact, I am very curious to know more about the journal and plan to submit the manuscript of one of my students. But I have become so busy now that I won't be able to help. I suggest that you invite my colleague @AlexandreWadoux.

Best.

vissarion commented 3 years ago

Thanks @samuel-rosa for your recommendation.

Hi, @AlexandreWadoux, would you be willing to review this submission for JOSS?

AlexandreWadoux commented 3 years ago

Hello @vissarion, sorry but as a principle I never review for journals publishing the name of the reviewers. I do not want to be held responsible if a poor paper of software package is accepted despite a recommendation to reject.

vissarion commented 3 years ago

@AlexandreWadoux I understand your concern. Thanks for the quick reply!

As a side note, in JOSS a submission is accepted only if all reviewers recommend acceptance according to those criteria https://joss.readthedocs.io/en/latest/review_criteria.html

vissarion commented 3 years ago

Hi, @glennon, @davyneven, @huanfachen, would you be willing to help reviewing this submission for JOSS?

ryankarlos commented 3 years ago

Hello @vissarion, thanks for approaching https://github.com/openjournals/joss-reviews/issues/3330#issuecomment-865692359 . Unfortunately I’m not going to have the time in the coming months so feel free to remove me from the list.

vissarion commented 3 years ago

Hello, @antoniomedrano, @NiklasAsselmann, @kramea would any of you be willing to help reviewing this submission for JOSS?

arfon commented 3 years ago

@vissarion I found you a second reviewer! @tmickleydoyle has kindly agreed to review this for us. Assuming that this is acceptable, I'll go ahead and set him up now...

arfon commented 3 years ago

@whedon add @tmickleydoyle as reviewer

whedon commented 3 years ago

OK, @tmickleydoyle is now a reviewer

arfon commented 3 years ago

@whedon re-invite @tmickleydoyle as reviewer

whedon commented 3 years ago

The reviewer already has a pending invite.

@tmickleydoyle please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

arfon commented 3 years ago

@tmickleydoyle – This is the review thread for the paper. All of our communications will happen here from now on.

Please read the "Reviewer instructions & questions" in the first comment above.

Both reviewers have checklists at the top of this thread (in that first comment) 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 https://github.com/openjournals/joss-reviews/issues/3330 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 the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

tmickleydoyle commented 3 years ago

πŸ‘‹ Thank you all for the invite

I was reading over the checklist of items but I am not able to interact with the markdown by checking the checklist items. I will continue to work on the checklist async. πŸ™‡

tmickleydoyle commented 3 years ago

Hello again πŸ‘‹

I was looking for the link to the developer docs so I could start reading and testing the different functionality of the code. Are they linked in the README of the repo?

Thank you πŸ™‡

jGaboardi commented 3 years ago

@tmickleydoyle Yes, that's the one: https://pysal.org/spopt/

tmickleydoyle commented 3 years ago

@jGaboardi πŸ‘‹ Thank you for the link, but I am still having a hard time finding the API docs.

jGaboardi commented 3 years ago

https://pysal.org/spopt/api.html

vissarion commented 3 years ago

@vissarion I found you a second reviewer! @tmickleydoyle has kindly agreed to review this for us. Assuming that this is acceptable, I'll go ahead and set him up now...

Thanks @arfon and sorry for the late reply, I was offline the last weeks.

arfon commented 3 years ago

Thanks @arfon and sorry for the late reply, I was offline the last weeks.

No worries at all. Hopefully this is a good outcome (finding a second reviewer 😁)

tmickleydoyle commented 3 years ago

Hey all. Hurricane Ida has impacted my access to the internet. I will try to get my review completed in the next two weeks. Sorry for the delay.

arfon commented 3 years ago

Hey all. Hurricane Ida has impacted my access to the internet. I will try to get my review completed in the next two weeks. Sorry for the delay.

Of course, no rush at all @tmickleydoyle. Please focus on life matters right now and work on the review when you are ready!

tmickleydoyle commented 2 years ago

Hey @jGaboardi πŸ‘‹

I opened an issue in the spopt repo about a challenge I am experiencing when trying to import MCLP. Thank you πŸ™‡

jGaboardi commented 2 years ago

@tmickleydoyle Thank you opening the issue. The reason the import is failing is because the locate module is not part of v0.1.1 release that is under review here. The original JOSS paper submission back in May (I believe) by @xf37 focused on regionalization because that was the only module in spopt at the time, with no locate functionality (the locate module is merged into code base but it is not in a release yet and is not published to PyPI or conda-forge). Since then there has been a v0.1.2 release and several major occurrences:

Due to these things I have some questions (and general musings) about moving forward with this submission:

Pinging some other PySAL core devs for thoughts & advice on this.

xf37 commented 2 years ago

@jGaboardi https://github.com/jGaboardi Thanks for taking care of this. I am back to work now. I am fine with both, either keeping this version or creating a new release. Just let me know your thoughts @gegen07 https://github.com/gegen07 (+ @qszhao https://github.com/qszhao & @ljwolf https://github.com/ljwolf) @sjsrey https://github.com/sjsrey, @knaaptime https://github.com/knaaptime

On Wed, Sep 15, 2021 at 6:45 PM James Gaboardi @.***> wrote:

@tmickleydoyle https://github.com/tmickleydoyle Thank you opening the issue. The reason the import is failing is because the locate module is not part of v0.1.1 release that is under review here. The original JOSS paper submission back in May (I believe) by @xf37 https://github.com/xf37 focused on regionalization because that was the only module in spopt at the time, with no locate functionality (the locate module is merged into code base but it is not in a release yet and is not published to PyPI or conda-forge). Since then there has been a v0.1.2 release and several major occurrences:

Due to these things I have some questions (and general musings) about moving forward with this submission:

  • Should we create a fresh release and rework the submission to include the locate module?
  • @xf37 https://github.com/xf37, do you still have time to spend addressing the concerns of the reviewers for this submission?
  • @gegen07 https://github.com/gegen07 @.*** https://github.com/qszhao & @ljwolf https://github.com/ljwolf), would you like to contribute to this paper if we rework it to include locate?
  • Also, I need to confirm with my current employer that I can still be involved with my current affiliation.

Pinging some other PySAL core devs for thoughts & advice on this.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3330#issuecomment-920465664, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEAWVDH3MLCNWM4NVWBRODTUCEV2LANCNFSM4567K6IQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

Dr. Xin Feng San Francisco, CA 94132, USA Email: @.***

jGaboardi commented 2 years ago

@arfon @vissarion Do you have any thoughts on this? @xf37 is OK for reworking the manuscript and we would like @gegen07 to get onboard to highlight his achievements in the locate module.

vissarion commented 2 years ago

@jGaboardi if locate module is the best way to address the review issue I agree to work on including it (either in this release or in a fresh one) and also update the JOSS paper text.

vissarion commented 2 years ago

@jGaboardi how major is the revision / rework you are working on? Does it make sense for a reviewer that didn't start the review yet to start it now or is it better to wait until you finish https://github.com/pysal/spopt/issues/196 ?

jGaboardi commented 2 years ago

@vissarion Thanks for touching base back with this! It would be better for that reviewer to wait until pysal/spopt#196 is complete. We are working on it now and will have the revisions merged and fresh release including the new functionality up soon.