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
Adding Sandbox notebooks #317
Conversation
-m All notebooks confirmed to run in the FrontierSI DEA Sandbox - Corrects decibel formula in Water_Classification.ipynb and Shipping_Lanes.ipynb - Fixes an instance of plot not displaying in do_it_yourself_notebook.ipynb - Numbers tutorial notebooks for ease of use - Updates README.md to reflect inclusion in GA Sandbox.
This are fantastic notebooks, and are really the level of documentation we should try and include in all notebooks in In the longer run (not necessary now):
|
I'm now working on running the notebooks in the GA sandbox (https://app.sandbox.dea.ga.gov.au) before we perform this PR. This comment is to log data/package issues that I run into. Case_Studies Notebooks:
Code_Examples Notebooks:
Dataset_Examples Notebooks:
Tutorial Notebooks:
High-level summary:
@alexgleith and @robbibt, could you please advise about how we should proceed to address each of these issues? Feel free to tag anyone else involved in the comments so we can keep track of what's happening. |
@caitlinadams I'm as of the last few hours working on updating the |
Regarding |
Hey folks
That notebook should be working with S2 NRT data... I'll check up on that.
It's much easier to get going than the Threddsky stuff :-)
…On Tue, 30 Jul 2019 at 13:50, Robbi Bishop-Taylor ***@***.***> wrote:
Regarding 02_Do_It_Yourself.ipynb with the missing Sentinel-2 data, I've
also run into that same problem. I believe that @alexgleith
<https://github.com/alexgleith> has plans to index the threddsky Sentinel
2 data which comes from the NCI, which should make the entire Sentinel 2
time series available for the notebook
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#317?email_source=notifications&email_token=AA2JIXNORGL375DDVM24LW3QB63BJA5CNFSM4IHBROT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3CVPYA#issuecomment-516249568>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA2JIXJZRENOLVPKVM4OSJLQB63BJANCNFSM4IHBROTQ>
.
--
Alex Leith
m: 0419189050
|
@alexgleith Regarding the other required datasets that look like they're missing or not indexed on the Sandbox... should we be recording this in any official place other than here? e.g. as issues on another repository, or an email to someone specific? |
@robbibt There wasn't any S2 NRT data indexed on DEA Sandbox .. I have started indexing S2 NRT along with Sentinel 2 Definitive .. will update you here once the indexing is complete |
The dask issue should now be resolved. The GA sandbox is configured with a different name, so the client value is different. Dask should be working now, however, the calculation in the |
- Reorganises structure and adds clear headings - Reformats output colour picture - Removes unnecessary libraries
- changes the client from FrontierSI key to DEA key
I've now confirmed that the data are available for all of the notebooks. Only remaining fixes are to get dask working on the sandbox, and to update any references of the DEA Dashboard (https://dashboard.dea-sandbox.test.frontiersi.io) to the DEA Explorer (https://explorer.dea.ga.gov.au). The Explorer needs to be updated to reflect the newly added data sets before this step can happen. Additionally, I've added explanatory text to the Code_Examples/Dask_Average_Colour_Australia.ipynb notebook (https://github.com/GeoscienceAustralia/dea-notebooks/blob/caitlinadams/DEA_sandbox/Code_Examples/Dask_Average_Colour_Australia.ipynb). It would be great if someone could review this as part of this PR. |
- Removes redundant .DS_Store file
I've now marked this as ready for review. Happy to take any feedback on text/code inside notebooks before this is merged. |
Note: we've deliberately not including some dea-notebook requirements like tags and index files because this needs some longer term thinking to work out how we want these AWS notebooks to show up on the user guide. For now, I think we can relax those rules for this pull request. The overall intention is that this new directory will be synced directly to the sandbox and will be the first thing people see when they log in. |
@caitlinadams I'll leave my small comments here as I go through the notebooks. In the agricultural case study, I think this line is incorrect "It takes values from -1 to 1, with high values corresponding to dense vegetation" - it should be "It takes values from -1 to 1, with high values corresponding to healthy vegetation". In the coastal erosion case study, the hyperlink in the description at the top doesn't work - 404 error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the Coastal_Erosion notebook. Under the heading for Computer MNDWI, there is a sentence where some words are missing: "When it comes to interpreting the index, , while."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than a couple of very small comments, these look great!
What is the purpose of duplicating the BandIndices.py
and utils.py
files in both the Case Studies and Tutorials folders?
Because of how Python accesses other files, they have to be contained within the folder of the script that's accessing them. This could be fixed if utility functions were included as a Python package instead, but I think that's a significant amount of development. |
@CEKrause That's a really good point... I guess the sandbox will need those scripts to run the notebooks, but it will only be reading in the |
@caitlinadams I like the text too, but I think we should probably get rid of it to reduce confusion when it doesn't render |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@
* Initial commit for DEA sandbox notebooks. * Minor changes to address GA Style -m All notebooks confirmed to run in the FrontierSI DEA Sandbox - Corrects decibel formula in Water_Classification.ipynb and Shipping_Lanes.ipynb - Fixes an instance of plot not displaying in do_it_yourself_notebook.ipynb - Numbers tutorial notebooks for ease of use - Updates README.md to reflect inclusion in GA Sandbox. * Changes dask client address * Renames and adds text to Dask example notebook - Reorganises structure and adds clear headings - Reformats output colour picture - Removes unnecessary libraries * Fixes dask client for sandbox - changes the client from FrontierSI key to DEA key * Refactor coastal erosion notebook to use Collection 3 data * Update waterline funcs load_cloudmaskedlandsat func to use Collection 3 data * Add tidal data for Gold Coast erosion study area * Minor fix to notebook to remove references to non-used bands * Clears evaluated cells in Coastal_Erosion notebook * Updates instructions to use Gold Coast tide file and Explorer link * Add tidal file for Perth * Reupload Perth tides with sensible name * Delete perth_-32.046331_115.716678_tides.csv * Delete dalyriver_-13.32_130.23_tides.csv * Delete josephbonapartegulf_-14.95_129.54_tides.csv * Delete pointstuart_-12.21_131.82_tides.csv * Updates references to Explorer and dask resource page - Removes redundant .DS_Store file * Adds references to dask resourcing page
* Initial commit for DEA sandbox notebooks. * Minor changes to address GA Style -m All notebooks confirmed to run in the FrontierSI DEA Sandbox - Corrects decibel formula in Water_Classification.ipynb and Shipping_Lanes.ipynb - Fixes an instance of plot not displaying in do_it_yourself_notebook.ipynb - Numbers tutorial notebooks for ease of use - Updates README.md to reflect inclusion in GA Sandbox. * Changes dask client address * Renames and adds text to Dask example notebook - Reorganises structure and adds clear headings - Reformats output colour picture - Removes unnecessary libraries * Fixes dask client for sandbox - changes the client from FrontierSI key to DEA key * Refactor coastal erosion notebook to use Collection 3 data * Update waterline funcs load_cloudmaskedlandsat func to use Collection 3 data * Add tidal data for Gold Coast erosion study area * Minor fix to notebook to remove references to non-used bands * Clears evaluated cells in Coastal_Erosion notebook * Updates instructions to use Gold Coast tide file and Explorer link * Add tidal file for Perth * Reupload Perth tides with sensible name * Delete perth_-32.046331_115.716678_tides.csv * Delete dalyriver_-13.32_130.23_tides.csv * Delete josephbonapartegulf_-14.95_129.54_tides.csv * Delete pointstuart_-12.21_131.82_tides.csv * Updates references to Explorer and dask resource page - Removes redundant .DS_Store file * Adds references to dask resourcing page
This pull request will add notebooks developed for the DEA Sandbox to the general notebook repository for DEA.
Still to do:
Happy to take any other feedback and update as necessary! I'll leave this as a draft PR until we're ready.