Skip to content
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

[PRE REVIEW]: pvpumpingsystem: a python package for modeling and sizing photovoltaic water pumping systems #2361

Closed
whedon opened this issue Jun 18, 2020 · 42 comments

Comments

@whedon
Copy link

whedon commented Jun 18, 2020

Submitting author: @tylunel (Tanguy Lunel)
Repository: https:/tylunel/pvpumpingsystem
Version: v1.0
Editor: @kbarnhart
Reviewers: @samuelduchesne, @robinroche
Managing EiC: Arfon Smith

⚠️ JOSS reduced service mode ⚠️

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.

Author instructions

Thanks for submitting your paper to JOSS @tylunel. Currently, there isn't an JOSS editor assigned to your paper.

@tylunel if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
@whedon
Copy link
Author

whedon commented Jun 18, 2020

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.

⚠️ JOSS reduced service mode ⚠️

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.

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
Copy link
Author

whedon commented Jun 18, 2020

Software report (experimental):

github.com/AlDanial/cloc v 1.84  T=0.11 s (233.6 files/s, 54242.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          20            971           2025           2811
Markdown                         2             26              0             83
YAML                             3             19             10             73
TeX                              1              2              0             17
-------------------------------------------------------------------------------
SUM:                            26           1018           2035           2984
-------------------------------------------------------------------------------


Statistical information for the repository '2361' was gathered on 2020/06/18.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Lunel                          126         27615           9128           67.90
tylunel                         55          2297          15071           32.10

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
Lunel                      4404           15.9          5.0               10.74
tylunel                    1403           61.1          1.8               26.80

@whedon
Copy link
Author

whedon commented Jun 18, 2020

Reference check summary:

OK DOIs

- 10.21105/joss.00884 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@arfon
Copy link
Member

arfon commented Jun 18, 2020

👋 @tylunel - Thanks for your submission to JOSS. As described in our blog post announcing the reopening of JOSS, we're currently working in a "reduced service mode", limiting the number of papers assigned to any individual editor.

Since reopening JOSS earlier in the month we've had a very large number of papers submitted and as such, yours has been put in our backlog that we will be working through over the coming weeks and months.

Thanks in advance for your patience!

@whedon
Copy link
Author

whedon commented Jun 18, 2020

@arfon arfon added the waitlisted Submissions in the JOSS backlog due to reduced service mode. label Jun 18, 2020
@danielskatz
Copy link

👋 @kbarnhart - would you be willing to edit this submission?

@danielskatz
Copy link

@whedon invite @kbarnhart as editor

@whedon
Copy link
Author

whedon commented Jun 19, 2020

@kbarnhart has been invited to edit this submission.

@kbarnhart
Copy link

@danielskatz I'm overloaded at the moment, but expect to be able to take on 1-2 new joss submissions by the end of next week. Happy to take this one on at that point if it is still in need of an editor.

@danielskatz
Copy link

Ok, let's see where we are next week.

@danielskatz
Copy link

👋 @kbarnhart - can you now take this one on?

@kbarnhart
Copy link

@danielskatz yes, I can.

@kbarnhart
Copy link

@whedon assign @kbarnhart as editor

@whedon
Copy link
Author

whedon commented Jun 29, 2020

OK, the editor is @kbarnhart

@danielskatz danielskatz removed the waitlisted Submissions in the JOSS backlog due to reduced service mode. label Jun 30, 2020
@kbarnhart
Copy link

@danielskatz thanks for removing the tag.... I forgot.

@kbarnhart
Copy link

kbarnhart commented Jun 30, 2020

@tylunel thank you for your submission to JOSS. I've placed the paused label on this because there are a couple of issues that need to be addressed before the submission can move forward in the JOSS review process. I've outlined these below with a checklist. If you have questions or clarifications about these issues, please comment here and I'll do my best to provide guidance.

  • Installation instructions should include how to install pvpumpingsytem itself.

  • Documentation should be expanded (see Review Criteria: Documentation). This part of the JOSS review criteria documentation states:

    "There should be sufficient documentation for you, the reviewer to understand the core functionality of the software under review. A high-level overview of this documentation should be included in a README file (or equivalent). "

    You are encouraged to accomplish this however you think is best. A common approach is a ReadTheDocs page. Given your extensive docstrings, this is likely the easiest solution.

    I would recommend that this documentation include the following three elements:

    • a narrative presentation of the package (statement of the core functionality, types of problems to be addressed, and/or design of the package),
    • the package API, and
    • example usage.
  • State how a user might test the software.

@kbarnhart
Copy link

@tylunel I wanted to follow up and see if you had any questions or clarifications about my comments.

@tylunel
Copy link

tylunel commented Jul 8, 2020

Hi @kbarnhart, thank you for your review!
Actually I did not expect the first review to be done so quickly, thanks for that! And the reason why I am not very responsive now is that I am currently on vacation until july 15 haha. So I will really take over the submission only then, sorry for the inconvenience.
Your comments seem clear so far, just maybe one question: for the third checkbox "State how a user might test the software", do you mean "how a user can run the tests of the software" or more "how the user can have a glimpse of what she/he can do with the software"?
Thank you very much!

@kbarnhart
Copy link

@tylunel no problem. Enjoy your vacation and let me know if you have additional questions for me when you return to this.

By "State how a user might test the software", I mean your first option. To state how a user can run the tests of the software. In the case of your contribution, I can see that there are test, just no instructions on how a person would run them.

It also looks like your CI badge only goes to an image of the badge, rather than a record of the CI.

@kbarnhart
Copy link

@tylunel I wanted to check in again and see if you were planning on revising pvpumpingsystem for review at JOSS.

@tylunel
Copy link

tylunel commented Aug 4, 2020

@kbarnhart Definitely! Sorry for the delay, the last three weeks haven't given me as much time as I expected to deal with it, but I start to work on it tomorrow. pvpumpingsystem should be ready for a second review by the end of this week.

@tylunel
Copy link

tylunel commented Aug 7, 2020

@kbarnhart I revised my package and added documentation with readthedocs. In my opinion, pvpumpingsystem is ready to be reviewed now.

When you wrote that the docs should include examples, do you mean like jupyter notebook files? So far, I had put 2 normal python files (.py) in the folder docs/examples with extensive docstring, is it enough?

@kbarnhart
Copy link

@tylunel thanks for making these improvements to pvpumpingsystem. However, there is still a bit more work that needs to be done before the package can move on to the review process.

Specifically, I think that the documentation and examples still need improvement.

  • Narrative documentation: I think the Package Overview section you've written is a wonderful addition to the documentation, but would benefit from one or two more sentences describing the scope of problems pvpumpingsystem is intended to solve. A prospective user should be able to quickly use this information to determine whether they want to use pvpumpingsystem in their application.
  • API documentation: The readthedocs page presently lists classes, functions, and methods provided by the package, but no information is provided regarding the inputs, outputs, and/or methods of each function and/or class. I see that you have documented much of this information in the source code docstrings, but I would expect to see this information compiled in the documentation pages. In addition, there are a number of instances in which an input is a path (e.g., for Pump, or for PVGeneration), but there is no documentation about the expected file format. The API level documentation should describe to a user the required inputs and outputs of all (best practice) or core (good practice) functions, classes, and methods.
  • Examples: JOSS has no requirements regarding the specific form of examples (e.g., python scripts vs. notebooks) in order to support developers in creating examples they think work best for their code. However, I would expect that the examples would provide more background of the problem being solved and description of what the code is doing. When I look at the two example scripts, it takes me quite a while to figure out what exactly the problem being solved is. Adding text around the code (as comments) that introduces the problem and explains how pvpumpingsytem can be used to solve it is necessary. I would also recommend making it easy for a user to see the examples (e.g., direct link from a read the docs page to the python script, notebook viewer, or binder).

If you have questions or clarifications about these issues, please comment here and I'll do my best to provide guidance.

@tylunel
Copy link

tylunel commented Aug 21, 2020

@kbarnhart thank you for the recommandations!
I reworked the structure and the content of my documentation, and I think it was worthwhile indeed. I convert the examples into jupyter notebook files, and significantly expanded the explanations around (I had never used Jupyter notebooks before, so I may not have used it optimally and any advice/feedback is welcomed).
The other points were also adressed, tell me if it suits your expectations now :)

@kbarnhart
Copy link

@tylunel thanks for making these changes. I definitely think that expanding your examples into notebooks and adding explanatory text substantially improves the package documentation. I also tried out the binder instance and it worked well! I find Binder is a great way to allow potential users to try out software quickly. Nice work 👍

Perhaps I'm mistaken, but I still don't see API documentation that describes the inputs and outputs of functions and classes.
I'm looking here in the documentation.

Annotation 2020-08-24 10453311

I see every function/class listed, but there is no information about what each does, what inputs are required and what outputs are provided. I want to make sure that I'm communicating my request clearly so I'll provide an example of what I'm looking for.

This numpy function for the identity matrix has the following documentation:

Annotation 2020-08-24 104533

It was built based on the docstring:

Annotation 2020-08-24 1045331

A common approach to making this sort of documentation is to use sphinx autodoc, formatted either based on the Numpy or Google convention (I've been pleased with using the napoleon sphinx extension for this). I also want to be clear that you don't need to use any specific method or set of tools to accomplish this (e.g., you don't need to use sphinx autodoc because I gave it as an example), but you do need to provide API level documentation for the package in some form.

If you have any questions or clarifications for me, please let me know and I'll do my best to provide guidance.

@tylunel
Copy link

tylunel commented Aug 25, 2020

Ho sorry! Actually you were very clear on what you expected, and I did the changes accordingly. But it seems that the documentation builds correctly only locally, and not when I push it on GitHub and readthedocs. So I'm embarrassed, I forgot to check that my last changes on API build well online too... I will investigate why local and online builds differ on that.

@kbarnhart
Copy link

@tylunel no problem. Whenever the API builds are live online, just ping me here and I'll take a look.

@tylunel
Copy link

tylunel commented Aug 31, 2020

@kbarnhart Now it should be okay!
As often in this kind of case, the problem was quite stupid but it still took me a long time to find out where it came from... Anyway, the API online is now as expected.

@kbarnhart kbarnhart removed the paused label Sep 1, 2020
@kbarnhart
Copy link

Nice work @tylunel. Glad you were able to get the docs to build on readthedocs! They look great and I think this is a huge improvement already!

I've now un-paused this submission and will begin the stage of the JOSS review process in which I find two or three reviewers for this submission.

If you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @, so you would refer to me as @ kbarnhart instead of @kbarnhart). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

I will incorporate your recommendations into my identification of reviewers.

@tylunel
Copy link

tylunel commented Sep 2, 2020

Thank you!
I would specially recommend @ motahhir who seems to already work on PV pumping systems. Then, from the 'topic/areas' column, @ kumar and @ robinroche could fit as well. Lastly, @ wholmgren may be also interested as pvpumpingsystem uses his package pvlib-python, and that some of the structure/philosophy of my package was inspired by his.

@kbarnhart
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Sep 2, 2020

@kbarnhart
Copy link

👋 @wholmgren @motahhir @samuelduchesne @robinroche would 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

This submission is for "pvpumpingsystem: a python package for modeling and sizing photovoltaic water pumping systems".

This is a pre-review issue which is used to find reviewers. Once 2-3 reviewers have been found I'll start the review on a dedicated GitHub issue. At present we are asking reviewers to complete reviews in 6 weeks. If you are not able to review and have someone to recommend, please mention them here (when mentioning, please place a space after the @ of a github handle, for example, you would refer to me as "@ kbarnhart").

If you are interested in reviewing, I would recommend looking over the journal's conflict of interest policy before accepting the review invitation.

If you have any questions about the JOSS review process, please do not hesitate to reach out to me by commenting on this issue or emailing me directly at [email protected]

@samuelduchesne
Copy link

Sure, I can review that.

@robinroche
Copy link

@kbarnhart Yes, I can also review this submission.

@kbarnhart
Copy link

@whedon add @samuelduchesne as reviewer

@whedon
Copy link
Author

whedon commented Sep 3, 2020

OK, @samuelduchesne is now a reviewer

@kbarnhart
Copy link

@whedon add @robinroche as reviewer

@whedon
Copy link
Author

whedon commented Sep 3, 2020

OK, @robinroche is now a reviewer

@kbarnhart
Copy link

@samuelduchesne and @robinroche thank you for your willingness to review. I will now start the main issue thread.

If one of the other invited reviewers would like to provide a review as well, let me know here and I can add you as a reviewer too.

@kbarnhart
Copy link

@whedon start review

@whedon
Copy link
Author

whedon commented Sep 3, 2020

OK, I've started the review over in #2637.

@whedon whedon closed this as completed Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants