[REVIEW]: ipcc - a Python package for the calculation of national greenhouse gas inventories

editorialbot commented 9 months ago

Submitting author: !--author-handle-->@mabudz@pdebuyl<!--end-editor-- Reviewers: @GISWLH, @PennyHow Archive: 10.5281/zenodo.10822520

Status

Status badge code:

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

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

@GISWLH & @PennyHow, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

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

Checklists

📝 Checklist for @PennyHow

📝 Checklist for @GISWLH

editorialbot commented 9 months ago

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

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

@editorialbot commands

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

@editorialbot generate pdf

editorialbot commented 9 months ago

Software report:

github.com/AlDanial/cloc v 1.88  T=0.14 s (443.9 files/s, 157705.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
YAML                             4             26             39           7975
Python                          40           1806           2976           6355
Markdown                        13            163              0            506
Jupyter Notebook                 3              0           2461            230
INI                              1             11              0             81
TeX                              1              5              0             63
make                             1              6              8             15
TOML                             1              1              3              5
-------------------------------------------------------------------------------
SUM:                            64           2018           5487          15230
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 9 months ago

Wordcount for paper.md is 1504

editorialbot commented 9 months ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1111/jiec.12713 is OK
- 10.1111/jiec.12715 is OK
- 10.1080/07350015.2015.1038545 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 9 months ago

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

PennyHow commented 9 months ago

Review checklist for @PennyHow

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://gitlab.com/bonsamurais/bonsai/util/ipcc?
[x] License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@mabudz) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

GISWLH commented 9 months ago

Review checklist for @GISWLH

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://gitlab.com/bonsamurais/bonsai/util/ipcc?
[x] License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@mabudz) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[ ] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[ ] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

pdebuyl commented 9 months ago

@GISWLH @PennyHow please check the information text at the top of this page (@PennyHow I see this is already done, thanks for starting!).

As the review takes place into this discussion page, feel free to provide a partial review at some point, to ask clarifications to the author here, or to ask me about the review process.

Some reviewers also open issues at the repository under review. This is fine for bug reports or requests about the documentation, for instance. In that case, please report the resulting bugfix or other result from the discussion here so that I can get proper notification.

GISWLH commented 9 months ago

Thank you, @pdebuyl, for your invitation and kind reminder 👋. I will review this paper and its code in the coming days. I guess i will raising issues in the ipcc repository. Once completed, a summary of my observations will also be provided here.

pdebuyl commented 9 months ago

Hi @GISWLH I see that you ticked all the boxes already. Their purpose is to indicate the progress of your review, which I guess is not yet over. Can you untick them please?

GISWLH commented 9 months ago

Sorry for the misunderstanding and i have now unticked the option. @pdebuyl

PennyHow commented 9 months ago

Hi @mabudz and @pdebuyl, here is my initial review: in all, I think this is a useful package that upholds the idea of reproducibility and accessibility to an important set of IPCC data and calculations. I have some comments on how it is presented and distributed that need to be resolved before it should be accepted from my side:

[x] The JOSS paper itself looks in good shape. I've made some minor corrections in a merge request here, along with suggested changes to the repo readme and documentation.
[x] I see you note in the readme that the pypi package distribution is not ready yet. I would like to see the pypi package finalised (and usable) as part of this review process.
[x] On a related note to the point above, can the example scripts also be distributed with the pypi package build? I would like to see the example included and incorporated into the testing of the package installation, perhaps as part of some unit testing for the package. I also see a tests/ directory with many scripts - do you use these in your examples in the documentation and/or the package distribution?
[x] There are many tutorials in your documentation that merely perform look-ups from the .csv tables. I would like to see more tutorials in the documentation that demonstrate the functionality of the ipcc package, such as calculating GHG inventories and inter-comparison of the different uncertainty calculation methods. Without these examples, it is difficult to see the capabilities of the package beyond just a library of look-up tables.
[x] The contributions section thoroughly describes where a contributor could contribute to the ipcc package. However, there is little description on how a contributor should contribute (e.g. merge request from a forked branch, with a clear description of the contributions?). Please outline this in the CONTRIBUTING.md file.
[x] Please specify somewhere in the installation guide that the ipcc package requires python>=3.9. In fact, I think it would be good to include a complete installation guide, so something like this:

conda create --name py311 python=3.11
conda activate py311
git clone git@gitlab.com:bonsamurais/bonsai/util/ipcc.git
cd ipcc
pip install -e .
python3
import ipcc

[x] Two minor changes I have opened as issues: tier sequence example, undefined variable in example

GISWLH commented 8 months ago

Dear @pdebuyl and @mabudz, this is my review report: In my opinion, this package provides tools for national greenhouse gas calculation and takes into account uncertainty from IPCC guidelines. It holds potential applicative value for climate change scientists and aligns with the interests and scope of the JOSS journal. However, I have the following suggestions for its improvement:

[ ] I am uncertain about the appropriateness of the author's use of the organizational function format ipcc.<volume>.<chapter>. With the release of new versions of the IPCC reports, the names of volumes and chapters may change.
[ ] The authors should provide more background information in the repository. It should include PDFs of some IPCC chapters (e.g., https://www.ipcc.ch/site/assets/uploads/2018/03/5_Waste-1.pdf), as these contain the original formulas for users to verify.
[ ] I wonder if it's appropriate for the author to use ipcc as the name for a Python library, given that IPCC is a non-profit organization dedicated to climate change science, encompassing not just greenhouse gas calculations but also climate change, regional imbalances, emission scenarios, hydrology, and water cycles, among other topics. Considering the actual functionality of the author's library, a name like ipccgas might be more suitable. Of course, this is just a suggestion for the author to consider, as the library is their development.
[ ] The current version may not be sufficiently user-friendly, as users might not know how to apply their data for uncertainty analysis. I didn't find adequate guidance on how to apply my data. The authors should clearly provide specific steps or a simple example.
[ ] Some parts of the README code cannot be executed directly. The author should carefully check to avoid errors. For example, in the 'Add own data' section, swd.parameter.r = df should be corrected to my_ipcc.waste.swd.parameter.r = df.

mabudz commented 8 months ago

Dear @PennyHow, @GISWLH and @pdebuyl , thanks a lot for the excellent comments!

I have revised the package and merged already to the main branch (The manuscript is still on the joss branch). The following has been changed:

The package is available on PyPi as bonsai-ipcc. Regarding the suggestion for changing the name, I think this also avoids confusion in terms of the official IPCC organization. However, the package name is still ipcc.
I revised the tutorials. The previous versions, indeed, had a lot of dataframe manipulation which made it difficult to identify the main functionalities. By restructuring the jupyter tutorials into basic concept, core functionality and uncertainty I hope this is more obvious, especially in terms of adding own data with uncertainty information. I also deleted the folder scripts/ which was not intended to be in the package.
The readme.md has been revised to provide a full working example. Furthermore, I added additional information linking to the pdf documents of the IPCC guidelines.
The contributing.md has been revised to provide more detailed instructions (e.g., create issue, create branch, merge request etc). Also regarding the required python version >=3.9

I still think that following the structure of volume and chapter makes sense. The reason is that this should help users to find the specific information about equations, tier sequences and data also in the original pdf documents. The IPCC guidelines were published in 2006 and got and update in 2019 without changing the original structure of volume and sequence. For further updates I hope they follow the same logic.

pdebuyl commented 8 months ago

Thanks @PennyHow @GISWLH for the reviews and recommendations!

Regarding the distribution of examples with the software package, it is not a widespread practice so I don't believe that it can be essential as a review item. @mabudz you are free to take it into account of course. @PennyHow do you have examples of how you would see this achieved?

Regarding the package name, it could be indeed beneficial to the package to avoid any confusion. bonsai_ipcc woud fit the PyPI name (bonsai-ipcc cannot be imported).

I have a remark on the README part on installing the package. Only developers should install with the -e option to pip -> else the package is at risk of being modified in place thus modifying the results of the calculations.

mabudz commented 8 months ago

Thanks @pdebuyl, The pypi name is indeed bonsai_ipcc.

The readme has now two separate instructions for users and developers. Only for developers pip install -e is recommended.

PennyHow commented 8 months ago

Regarding the distribution of examples with the software package, it is not a widespread practice so I don't believe that it can be essential as a review item. @mabudz you are free to take it into account of course. @PennyHow do you have examples of how you would see this achieved?

I think with the newly updated examples in the documentation, they do not need to be included with the package distribution.

Regarding the package name, it could be indeed beneficial to the package to avoid any confusion. bonsai_ipcc woud fit the PyPI name (bonsai-ipcc cannot be imported).

I agree with this. To install, the pypi distribution is named bonsai-ipcc (i.e. install as pip install bonsai-ipcc). But to import, the package name remains ipcc (i.e. import ipcc). I think the import name should also be bonsai-ipcc to avoid potential conflicts with other packages.

All the other comments from me have been adequately addressed.

mabudz commented 8 months ago

I have changed the package name. To avoid conflicts bonsai_ipcc is used for both the package name and the name on pypi. I have revised the manuscript in this regard.

mabudz commented 8 months ago

@editorialbot generate pdf

editorialbot commented 8 months ago

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

mabudz commented 7 months ago

Dear @pdebuyl , @PennyHow , @GISWLH ,

Many thanks for your recommendations, I have tried to address all of those. Please let me know if you have any further recommendations.

PennyHow commented 7 months ago

Many thanks for your recommendations, I have tried to address all of those. Please let me know if you have any further recommendations.

Nothing else from me. You have adequately addressed all of my comments 👍🏼

GISWLH commented 7 months ago

Many thanks for your recommendations, I have tried to address all of those. Please let me know if you have any further recommendations.

Upon reviewing the revisions you've implemented based on my recommendations, I am pleased to endorse this manuscript for publication.

pdebuyl commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

pdebuyl commented 6 months ago

@editorialbot check references

editorialbot commented 6 months ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1111/jiec.12713 is OK
- 10.1111/jiec.12715 is OK
- 10.1080/07350015.2015.1038545 is OK

MISSING DOIs

- None

INVALID DOIs

- None

pdebuyl commented 6 months ago

Hi authors,

Sorry (a lot) for the delay. Too many demands at "${MAINJOB}" :-/

Here is my feedback for the article:

Link for "Schmidt, J., Merciai, S., Munoz, I., Rosa, M. de, & Astudillo, M. (2021). The big207 climate database v1 – methodology report" is broken.
Please replace the ellipsis in Stadler 2018 by the actual author names
Please remove the example code from the article. Code samples belong to the documentation (see https://joss.readthedocs.io/en/latest/submitting.html the part "what should my paper contain" and the reference to API documentation). You can keep the calling line and describe the rest textually.
Typos: "statement of need" first sentence: environmnetal -> environmental . second sentence: "e.g. , " -> ", e.g. " . "Conclusion": reproducability -> reproducibility ; automatization -> automation ; implemenetation -> implementation.
Could you state whether Monte Carlo simulations are also used in IPCC reports for error propagation?
The use of the __dict__ attribute as the recommended method to show the result of the calculation is really unusual. Would you consider using __repr__ instead as shown below or something similar?
```
def __repr__(self):
    return self.__dict__
```
One information that we typically expect in a JOSS paper is missing in your article: are there similar packages already and if so how does bonsai_ipcc compare?

pdebuyl commented 6 months ago

@GISWLH Thank you very much for the review!

pdebuyl commented 6 months ago

@PennyHow Thank you very much for the review!

pdebuyl commented 6 months ago

PS: @mabudz it is nice to have changed the import name. A short name such as ipcc can lead to confusion and this will probably make it easier for your users and for referencing the software as well.

mabudz commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

mabudz commented 6 months ago

Hi @pdebuyl , many thanks for your feedback! According to your comments:

I fixed the link
The ellipsis is created by the latex implementation in the background. I have added all authors to the bib file. So the latex template would need to be changed to remove the ellipsis.
I removed the exemplary code. Only the lines of pseudo code are still included to illustrate the structure of the package.
Thanks a lot for correcting the typos. I have revised the manuscript.
I clarified in the summary section that both methods are suggested by the IPCC guidelines.
I am not fully sure about best practice. When calling repr(), however, it seems that a str should be returned. Thus I would add two methods to the class:
```
def __repr__(self):
    return str(self.__dict__)

def to_dict(self):
    return self.__dict__
```
I provide hestia_earth.models as example for implementing particular equations of the IPCC guidelines; and I compare it in the "statement of need" section.

pdebuyl commented 6 months ago

Hello,

Thanks for all the changes.

Regarding the ellipsis issue, I'll see with editor colleagues, as indeed the bib file is ok.

I have one extra typo line 97 magnitute -> magnitude

In the paragraph about uncertainty, can you change "ufloat with a link" to something more readable such as "the ufloat type from the frictionless library" and "a NumPy array" without link. In an article such inline links disrupt the reading. Also, for frictionless you can add it as a reference (the URL will do if they have no academic reference) and cite it. NumPy can also be cited (they have a recent-ish paper). Same for pandas.

The citation to hestia should be edited as well. It appears with "Project" as author which does not make much sense.

Regarding the __repr__ issue, you can use what makes most sense to you. But advising users to see the content of your data structure with __dict__ is weird. Underscored attributes are not meant to be visibly used by users.

I have only one remaining recommendation for the project itself: the repository is under the MIT license. You could consider putting the data files (csv) under CC-BY or similar for making it easier in case others want to re-use just that component. But that is up to you.

Sorry for the laundry list of citation issues. Once this is cleared, the paper will be ready to go.

mabudz commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

mabudz commented 6 months ago

Thanks a lot @pdebuyl ,

I have revised the citations according to your comments and fixed the typo.

The main branch includes a to_dict() method that can be used to show results as a dictionary.

Regarding the license for the data files, I will think about to consider adding such a license in a future release.

pdebuyl commented 6 months ago

Thanks @mabudz !

Now, can you make a zenodo archive, post here the zenodo DOI and the version number of that archived release, make sure that all zenodo metadata is correct (title must be equal to paper title, and author list match the JOSS article).

mabudz commented 6 months ago

DOI: 10.5281/zenodo.6405387

Version: v0.3.2

mabudz commented 6 months ago

but please use the joss branch or the main branch for creating the paper

pdebuyl commented 5 months ago

@editorialbot set 10.5281/zenodo.6405387 as archive

editorialbot commented 5 months ago

Done! archive is now 10.5281/zenodo.6405387

pdebuyl commented 5 months ago

Hi @mabudz the DOI does not seem to point to ipcc :-/

mabudz commented 5 months ago

Hi @pdebuyl , sorry for the confusion. 😅 This is the right one

10.5281/zenodo.10822520

pdebuyl commented 5 months ago

@editorialbot set 10.5281/zenodo.10822520 as archive

editorialbot commented 5 months ago

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

@editorialbot commands

pdebuyl commented 5 months ago

THanks @mabudz . Would Mathieu Delpierre have an ORCID?

pdebuyl commented 5 months ago

@editorialbot set 10.5281/zenodo.10822520 as archive

editorialbot commented 5 months ago

Done! archive is now 10.5281/zenodo.10822520

openjournals / joss-reviews