Review #2

acocac · 2022-08-11T08:30:18Z

Notebook Review: Issue#99

Notebook Name: Exploring Land Cover Data (Impact Observatory)

Submitting Author: @jamesdamillington

Pull Request: PR#110

Reviewer instructions & questions

@annefou @aedebus, please carry out your review in this PR by using the checklist template below (available for copy and paste here).

Any questions, concerns or suggestions regarding the review process contact the editor.

✨ Please start on your review when you are able, and be sure to complete your review within the next four weeks, thank you! ✨

Review Checklist

Code of conduct

I confirm that I read and will adhere to the Environmental Data Science code of conduct.

General checks

Notebook: Is the notebook file part of the PR?
Contribution and authorship: Does the author list seem appropriate and complete?
Scope and eligibility: Does the submission contain an original and complete analysis according to the theme selected?

Reproducibility

Does the notebook run in a local environment?
Does the notebook build and run in binder?
Are all data sources openly accessible and properly cited (e.g. with citation to a persistent DOI) in the heading section?

Pedagogy

Are the notebook purpose and highlights clear?
Does the notebook demonstrate some specific data analysis or visualisation techniques?
Is the notebook well documented, using both markdown cells and comments in code cells?
Does the conclusion section provide clear and concise final say on the tools, analysis and/or datasets used?
Is the notebook narrative well written (it does not require editing for structure, language, or writing quality)?

Ethical

Is any linkage of datasets in the notebook unlikely to lead to an increased risk of the personal identification of individuals?
Is the notebook truthful and clear about any limitations of the analysis (and potential biases in data and/or tools)?
Is the notebook unlikely to lead to negative social outcomes, such as (but not limited to) increasing discrimination or injustice?

Other Requirements

All mentioned software should be formally and consistently cited wherever possible.
Acronyms should be spelled out upon first mention.
Licence conditions on images and figures must be respected (Creative Commons, etc.).

AOB

Please use the ReviewNB to submit your reviews on the notebook. ReviewNB is a third-party plugin in GitHub for displaying Jupyter Notebook content.

In addition to ReviewNB, we suggest to explore or run the notebook in:

Binder (run): Click the Launch Binder button at the top level of this message.
Netlify (only previews): a preview hosted by Netlify can be also inspected in the pull request of the contribution (see here). Note the preview will only be updated when we merge the PR.

review-notebook-app · 2022-08-11T08:30:23Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

general-exploration-landcover_io.ipynb

annefou · 2022-08-11T13:26:47Z

general-exploration-landcover_io.ipynb

@@ -38,6 +38,9 @@
    "\n",


Line #4. study_poly = Polygon(zip(lon_points, lat_points))
Then here we would have:
tudy_poly = Polygon(study_bbox_coordinates)

Reply via ReviewNB

Edited to correspond to new construction of bbox coordinates

closing conversation, resolved

annefou · 2022-08-11T16:00:55Z

general-exploration-landcover_io.ipynb

@@ -38,6 +38,9 @@
    "\n",


The paths are not correct (also in the text above). We should have

general-exploration-landcover_io

instead of

general-exploration-landcover

Reply via ReviewNB

Sorry this was my bad. I changed the name of the project folder to general-exploration-landcover_io without veryfing where else it's used. I'll sort this issue out as part of pending task for the editor. Thanks @annefou

ok, fixed now see commit 9ab8466. For reproducibility, I'd suggest to use os.path.join to call the existing notebook_folder variable. For instance, change lcxr.to_netcdf("general-exploration-landcover/MT-study-area.nc") to lcxr.to_netcdf(os.path.join(notebook_folder,"MT-study-area.nc")). How does it sound @jamesdamillington? Same for other cells using notebook_folder.

Yes, I agree @acocac, os.path.join is better. I have edited in the code and in the markdown.

I have also added decode_coords="all" to the xarray.open_dataset call to ensure CRS info is read back in correctly which it wasn't previously.

closing conversation, resolved

review-notebook-app · 2022-08-24T17:52:34Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:33Z
----------------------------------------------------------------

To match the format of the other notebooks’ headings (see examples in the Exploration theme), I would suggest:

1. Adding the mention of the dataset used (Impact Observatory) in the ‘Purpose’ section

2. Renaming the ‘Dataset description’ section ‘Sensor description’ and detail the characteristics of the Impact Observatory dataset

3. Renaming the ‘Dataset’ section ‘Dataset originator/creator’

4. Adding a section ‘Dataset reference and documentation’ (e.g. using this document)

5. Adding a note on the creative license

jamesdamillington commented on 2022-09-09T11:22:11Z
----------------------------------------------------------------

Thanks for these useful suggestions. I have added information as suggested (although I am not sure 'Sensor description' is entirely appropriate as really the content describes datasets/products derived from sensors). For dataset reference and documentation I have added a peer review paper in addition to the technical document highlighted.

acocac commented on 2022-09-20T09:54:46Z
----------------------------------------------------------------

@jamesdamillington you're right dataset is the correct term for your notebook. We'll change it during the typesetting process if that's ok for you.

TODO in the typesetting (post-print) stage

review-notebook-app · 2022-08-24T17:52:35Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:34Z
----------------------------------------------------------------

Line #5.    km2deg = 1.0 / 111

I am not sure but it would maybe help clarify this part if a comment on where the number '111' comes from is added (e.g. 1 deg corresponds to approximately 111 m at the equator)

jamesdamillington commented on 2022-09-09T12:19:24Z
----------------------------------------------------------------

Yes, good idea. I have added this comment (with 111 km)

acocac commented on 2022-09-21T16:30:14Z
----------------------------------------------------------------

closing conversation, resolved

review-notebook-app · 2022-08-24T17:52:35Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:35Z
----------------------------------------------------------------

Here, I would maybe suggest adding a line interpreting this graph (e.g. 'we can see the whole study area represented by the red rectangle is included in the datasets queried represented by the blue rectangles')

jamesdamillington commented on 2022-09-09T12:19:32Z
----------------------------------------------------------------

Yes, I agree this would be useful. I have also added text prior to this to explain each dataset is for a tile of the earth's surface.

acocac commented on 2022-09-21T16:33:27Z
----------------------------------------------------------------

closing conversation, resolved

review-notebook-app · 2022-08-24T17:52:36Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:36Z
----------------------------------------------------------------

Just a thought: maybe it could be nice to duplicate the graphs that have been produced below for pixel counts for 'estimated area' as well using the 300 m res?

jamesdamillington commented on 2022-09-10T09:20:23Z
----------------------------------------------------------------

This is a good point, in that it is likely more intuitive to work in area rather than pixel counts. Duplicating the graphs seemed repetitive so instead I have changed the original graph to have units of area.

acocac commented on 2022-09-21T16:35:02Z
----------------------------------------------------------------

closing conversation, resolved

review-notebook-app · 2022-08-24T17:52:37Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:36Z
----------------------------------------------------------------

Same here as in 3: it might be interesting to add the visualisation in terms of area?

jamesdamillington commented on 2022-09-10T09:20:31Z
----------------------------------------------------------------

I have changed the sankey to show the transitions as units of area rather than cell counts. In the subsequent table, I now show both cell counts and area

acocac commented on 2022-09-21T16:36:13Z
----------------------------------------------------------------

closing conversation, resolved

review-notebook-app · 2022-08-24T17:52:37Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:37Z
----------------------------------------------------------------

Typo in 'socio-econmically' in the second paragraph
The hyperlink for the source might be better integrated with the 'zonal statistics' text in the first paragraph

jamesdamillington commented on 2022-09-09T12:19:43Z
----------------------------------------------------------------

Thanks, I have made these edits.

NHomer-Edi commented on 2022-09-21T16:34:29Z
----------------------------------------------------------------

closing conversation, resolved

review-notebook-app · 2022-08-24T17:52:38Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:38Z
----------------------------------------------------------------

Line #14.    axes.set_xlabel('x [metre]'), axes.set_ylabel('y [metre]')

To add labels for axes?

jamesdamillington commented on 2022-09-09T12:19:48Z
----------------------------------------------------------------

Thanks. I have added these labels

NHomer-Edi commented on 2022-09-21T16:35:13Z
----------------------------------------------------------------

closing converstation, resolved

review-notebook-app · 2022-08-24T17:52:39Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:39Z
----------------------------------------------------------------

Line #13.    sa_munis_xr = xr.DataArray(data=sa_munis_r, name='Municipality code'

Suggestion to understand the colorbar in the next plot better

jamesdamillington commented on 2022-09-09T12:19:52Z
----------------------------------------------------------------

Thanks, I have added this

NHomer-Edi commented on 2022-09-21T16:35:41Z
----------------------------------------------------------------

closing converstation, resolved

review-notebook-app · 2022-08-24T17:52:40Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:40Z
----------------------------------------------------------------

Line #18.    ax[ix].set_xlabel('x [metre]'), ax[ix].set_ylabel('y [metre]')

To add labels for axes?

jamesdamillington commented on 2022-09-09T12:19:56Z
----------------------------------------------------------------

Thanks. I have added these labels

NHomer-Edi commented on 2022-09-21T16:36:00Z
----------------------------------------------------------------

closing converstation, resolved

review-notebook-app · 2022-08-24T17:52:41Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:40Z
----------------------------------------------------------------

Line #18.   ax[ix].set_xlabel('x [metre]'), ax[ix].set_ylabel('y [metre]')

To add labels for axes?

jamesdamillington commented on 2022-09-09T12:20:00Z
----------------------------------------------------------------

Thanks. I have added these labels

NHomer-Edi commented on 2022-09-21T16:36:19Z
----------------------------------------------------------------

closing converstation, resolved

review-notebook-app · 2022-08-24T17:52:42Z

View / edit / reply to this conversation on ReviewNB

aedebus commented on 2022-08-24T17:52:41Z
----------------------------------------------------------------

Details of the dataset version to add here in 'Dataset'

jamesdamillington commented on 2022-09-10T09:29:51Z
----------------------------------------------------------------

Thanks, this is now added

aedebus commented on 2022-09-19T14:46:13Z
----------------------------------------------------------------

There is a small typo here with an extra "

acocac commented on 2022-09-20T10:18:48Z
----------------------------------------------------------------

TODO in the typesetting stage.

aedebus · 2022-08-24T17:58:08Z

cm-template-review-checklist)=

Review Checklist Template

:::{note}
The checklist template follows the suggested review criteria for SciPy Proceedings and
Turing Data Stories
:::

Review Checklist : First review, @aedebus, 24-08-2022

Code of conduct

I confirm that I read and will adhere to the Environmental Data Science code of conduct.

General checks

Notebook: Is the notebook file part of the PR?
Contribution and authorship: Does the author list seem appropriate and complete?
Scope and eligibility: Does the submission contain an original and complete analysis according to the theme selected?

Reproducibility

Does the notebook run in a local environment?
Comment: Issue in instructions in ReadME : conda env create -f environment.yml + changes needed in paths when loading local data (already done in review_round1 branch)
Does the notebook build and run in binder?
Are all data sources openly accessible and properly cited (e.g. with citation to a persistent DOI) in the heading section?
Comment: See ReviewNB suggestions to improve this in the heading section

Pedagogy

Are the notebook purpose and highlights clear?
Does the notebook demonstrate some specific data analysis or visualisation techniques?
Is the notebook well documented, using both markdown cells and comments in code cells?
Comment: See ReviewNB comments for some suggestions
Does the conclusion section provide clear and concise final say on the tools, analysis and/or datasets used?
Is the notebook narrative well written (it does not require editing for structure, language, or writing quality)?

Ethical

Is any linkage of datasets in the notebook unlikely to lead to an increased risk of the personal identification of individuals?
Is the notebook truthful and clear about any limitations of the analysis (and potential biases in data and/or tools)?
Is the notebook unlikely to lead to negative social outcomes, such as (but not limited to) increasing discrimination or injustice?

Other Requirements

All mentioned software should be formally and consistently cited wherever possible.
Acronyms should be spelled out upon first mention.
Licence conditions on images and figures must be respected (Creative Commons, etc.).

AOB

acocac · 2022-08-25T08:47:37Z

@aedebus thanks for the first review 👍 Can you confirm if it is ready for its revision from the author @jamesdamillington?

acocac · 2022-08-25T08:49:49Z

@annefou, a friendly reminder that the deadline for first reviews is Thursday, September 8th. Where possible can you also use the suggested checklist template and confirm when your review is ready. Thank you 🙏

aedebus · 2022-08-25T09:31:15Z

@acocac @jamesdamillington I can confirm my review is ready for revision ✅

annefou · 2022-08-30T07:28:09Z

@annefou, a friendly reminder that the deadline for first reviews is Thursday, September 8th. Where possible can you also use the suggested checklist template and confirm when your review is ready. Thank you 🙏

I am working on it.

review-notebook-app · 2022-09-01T09:30:26Z

View / edit / reply to this conversation on ReviewNB

annefou commented on 2022-09-01T09:30:26Z
----------------------------------------------------------------

Line #7.        bbox=study_bbox

study_bbox is not defined anymore. We should have:

bbox=study_poly.bounds

jamesdamillington commented on 2022-09-09T12:20:17Z
----------------------------------------------------------------

Yes, thanks I have fixed this and elsewhere

NHomer-Edi commented on 2022-09-21T16:30:57Z
----------------------------------------------------------------

closing conversation, resolved

jamesdamillington · 2022-09-10T09:29:52Z

Thanks, this is now added

View entire conversation on ReviewNB

jamesdamillington · 2022-09-10T09:32:06Z

@acocac I think I have now responded to all reviewer comments in review round 1

acocac · 2022-09-13T11:11:14Z

@jamesdamillington thanks for going through each of reviewers comments!

Please @annefou @aedebus can you confirm if you're happy with all replies to your suggestions/comments. Feel free to reply each comment and/or update the checklist accordingly.

annefou · 2022-09-19T12:06:57Z

@jamesdamillington thanks for going through each of reviewers comments!

Please @annefou @aedebus can you confirm if you're happy with all replies to your suggestions/comments. Feel free to reply each comment and/or update the checklist accordingly.

I am happy with the updated notebook. I think it is great and on my side ready to be merged. Thanks.

aedebus · 2022-09-19T14:46:14Z

There is a small typo here with an extra "

View entire conversation on ReviewNB

aedebus · 2022-09-19T14:46:25Z

@jamesdamillington Thank you for implementing the changes. I have noticed a small typo and commented accordingly, but everything looks good to me otherwise.

@acocac I can confirm that other than this minor detail, it looks ready to be merged from my end too.

acocac · 2022-09-20T09:54:47Z

jamesdamillington you're right dataset is the correct term for your notebook. We'll change it during the typesetting process if that's ok for you.

View entire conversation on ReviewNB

acocac · 2022-09-20T09:58:35Z

@annefou this is an error in ReviewNB, the version in Binder only has a single colormap plotted.

View entire conversation on ReviewNB

acocac · 2022-09-20T10:18:49Z

TODO in the typesetting stage.

View entire conversation on ReviewNB

acocac · 2022-09-20T10:25:20Z

@jamesdamillington thanks for going through each of reviewers comments!
Please @annefou @aedebus can you confirm if you're happy with all replies to your suggestions/comments. Feel free to reply each comment and/or update the checklist accordingly.

I am happy with the updated notebook. I think it is great and on my side ready to be merged. Thanks.

@annefou thanks for recommending to merge the notebook and move to the post-print stage.

acocac · 2022-09-20T10:26:29Z

@jamesdamillington Thank you for implementing the changes. I have noticed a small typo and commented accordingly, but everything looks good to me otherwise.

@acocac I can confirm that other than this minor detail, it looks ready to be merged from my end too.

@aedebus thanks for confirming all is ok! The minor typos will be addressed in the post-print stage.

jamesdamillington · 2022-09-20T10:30:35Z

Great! Thanks all. Do I need to do anything now?

acocac · 2022-09-20T10:54:38Z

@jamesdamillington. You don't need to anything now. The notebook is good to go -- thanks to you and reviewers!

Next steps:

The moderators will go through each of the comments in ReviewNB and close them accordingly except TODOs for the post-print stage.
The editorial team will share proofs (the draft of the final formatting) using Netlify preview in the PR of the notebook in the main repo of the EDS book. We'll inform when it is ready through the Notebook idea issue #199.
We usually release and announce new EDS notebooks on Mondays at noon UTC. This means we expect to release it Monday next week or the week after (at the latest 🤞).

acocac · 2022-09-21T16:30:15Z

closing conversation, resolved

View entire conversation on ReviewNB

NHomer-Edi · 2022-09-21T16:30:58Z

closing conversation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:33:28Z

closing conversation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:33:43Z

closing conversation, resolved

View entire conversation on ReviewNB

NHomer-Edi · 2022-09-21T16:34:30Z

closing conversation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:34:44Z

closing conversation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:35:03Z

closing conversation, resolved

View entire conversation on ReviewNB

NHomer-Edi · 2022-09-21T16:35:14Z

closing converstation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:35:23Z

closing conversation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:35:40Z

closing conversation, resolved

View entire conversation on ReviewNB

NHomer-Edi · 2022-09-21T16:35:41Z

closing converstation, resolved

View entire conversation on ReviewNB

NHomer-Edi · 2022-09-21T16:36:01Z

closing converstation, resolved

View entire conversation on ReviewNB

acocac · 2022-09-21T16:36:14Z

closing conversation, resolved

View entire conversation on ReviewNB

NHomer-Edi · 2022-09-21T16:36:20Z

closing converstation, resolved

View entire conversation on ReviewNB

add reviewers

015b760

acocac assigned jamesdamillington Aug 11, 2022

acocac requested review from annefou and removed request for annefou August 11, 2022 08:30

crangelsmith mentioned this pull request Aug 11, 2022

Add Binder link to review PR alan-turing-institute/TuringDataStories#202

Open

2 tasks

jamesdamillington mentioned this pull request Aug 11, 2022

[NBI] Exploring Land Cover Data alan-turing-institute/environmental-ds-book#99

Closed

9 tasks

annefou reviewed Aug 11, 2022

View reviewed changes

acocac and others added 4 commits August 11, 2022 17:19

fix wrong name in notebook folder

9ab8466

use os for path, incl decode_coords

01c0c1b

add data folder to gitignore

0c92a87

modify construction of study area polygon

e34f5e9

acocac merged commit 836e645 into main Sep 21, 2022

acocac changed the title ~~Review - Round 1~~ Review Feb 16, 2023

acocac deleted the review_round1 branch February 16, 2023 16:44

Review #2

Review #2

Conversation

acocac commented Aug 11, 2022 • edited Loading

Notebook Review: Issue#99

Reviewer instructions & questions

Review Checklist

Code of conduct

General checks

Reproducibility

Pedagogy

Ethical

Other Requirements

AOB

review-notebook-app bot commented Aug 11, 2022

annefou Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

jamesdamillington Aug 15, 2022

Choose a reason for hiding this comment

acocac Sep 21, 2022

Choose a reason for hiding this comment

annefou Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

acocac Aug 11, 2022

Choose a reason for hiding this comment

acocac Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

jamesdamillington Aug 15, 2022

Choose a reason for hiding this comment

acocac Sep 21, 2022

Choose a reason for hiding this comment

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

review-notebook-app bot commented Aug 24, 2022 • edited Loading

aedebus commented Aug 24, 2022

Review Checklist Template

Review Checklist : First review, @aedebus, 24-08-2022

Code of conduct

General checks

Reproducibility

Pedagogy

Ethical

Other Requirements

AOB

acocac commented Aug 25, 2022

acocac commented Aug 25, 2022

aedebus commented Aug 25, 2022

annefou commented Aug 30, 2022

review-notebook-app bot commented Sep 1, 2022 • edited Loading

jamesdamillington commented Sep 10, 2022

jamesdamillington commented Sep 10, 2022

acocac commented Sep 13, 2022 • edited Loading

annefou commented Sep 19, 2022

aedebus commented Sep 19, 2022

aedebus commented Sep 19, 2022

acocac commented Sep 20, 2022

acocac commented Sep 20, 2022

acocac commented Sep 20, 2022

acocac commented Sep 20, 2022

acocac commented Sep 20, 2022

jamesdamillington commented Sep 20, 2022

acocac commented Sep 20, 2022

acocac commented Sep 21, 2022

NHomer-Edi commented Sep 21, 2022

acocac commented Sep 21, 2022

acocac commented Sep 21, 2022

NHomer-Edi commented Sep 21, 2022

acocac commented Sep 21, 2022

acocac commented Sep 21, 2022

NHomer-Edi commented Sep 21, 2022

acocac commented Sep 21, 2022

acocac commented Sep 21, 2022

acocac commented Aug 11, 2022 •

edited

Loading

annefou Aug 11, 2022 •

edited

Loading

annefou Aug 11, 2022 •

edited

Loading

acocac Aug 11, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Aug 24, 2022 •

edited

Loading

review-notebook-app bot commented Sep 1, 2022 •

edited

Loading

acocac commented Sep 13, 2022 •

edited

Loading