-
Notifications
You must be signed in to change notification settings - Fork 39
Update snowex access #96
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
base: main
Are you sure you want to change the base?
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
[I will move to ReviewNB and add pertinent things there!] I have a number of observations that I'll add here (the changes to the notebook were too many for an "inline" commenting/suggestion option unfortunately - but I may be missing how to do that) Starting with kudos:
Things that didn't quite work:
Image corrections:
Text corrections and suggestions:
(For more content ideas, I'll start a new comment and/or figure out how to do this better) |
A comment about colors in the figure (SnowEx over ASO data): I don't think it works have the same color ramp and trying to overlay one with transparency on the other. For one, any time transparency is used, it messes up what the legend color-matching can actually achieve. I suspect the way to go here is have two different legend bars, one for Snow Ex and one for ASO. You could use two entirely different color ramps or invert the ramp for one of the data sets (this would be the matching-by-contrast concept). Leave both ramps fully opaque and trust the view to visually interpolate what the ASO data underlying the SnowEx data might be. Not ideal, but this isn't the kind of figure one would put in a paper anyway... it's just to get a sense of things. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:25:57Z Only a thought, but would it be better to show the nsidc.org/data/<short_name> URL and then have the hyperlink be the doi? I am not sure about a best practice for this. andypbarrett commented on 2025-07-17T21:21:00Z We should get guidance on this frizatch commented on 2025-07-18T22:58:25Z I have an inquiry in with tech writers / USO. frizatch commented on 2025-07-21T16:01:24Z Due to the website unification project, it was decided that the doi urls should stay, so this is good. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:25:58Z Image not rendering andypbarrett commented on 2025-07-17T21:27:16Z The image was not in the repo. Fixed! |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:25:59Z have earthaccess in the initial sentence be andypbarrett commented on 2025-07-17T21:30:02Z Fixed |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:25:59Z I think it's good to see this example, but maybe add language that this is the quick way to go for one data set, but to programmatically expand use of a bounding box, see a later method (?) andypbarrett commented on 2025-07-17T21:38:08Z I have deleted this text box and added a similar but expanded explanation at the end of Method 3 |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:00Z Change last sentence of first text section to ~ : Then select "Data Access Tool" from the cards provided. or Then select "Data Access Tool" from the options displayed.
Image of Data Access Tool GUI needs updating andypbarrett commented on 2025-07-17T21:45:52Z Fixed. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:00Z Is this dependent on someone drawing a polygon cw vs ccw? Is that the question here? I have not tested this (yet) but I'm curious. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:01Z Note: I am seeing 2 files, but I believe that is due to my login allowing me to see the ECS copy. I don't think this duplication of files affects how the rest of the notebook works for me. andypbarrett commented on 2025-07-17T22:04:47Z That is probably the case. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:02Z Change: Now that we have a bounding box for the SnowEx17 GPR we can start to look for ASO and MODIS data. First, ... To: Now that we have a bounding box for the SnowEx17 GPR we’ll be able to use it to look for coincident ASO and MODIS data. But first… andypbarrett commented on 2025-07-17T22:11:54Z Fixed |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:02Z Might be good to mention the output here is going from year > month > day > hour > minute > seconds... that is, the latter numbers are seconds for the end at least (why no secs for begin were output? - it's a bit confusing at first glance) andypbarrett commented on 2025-07-17T22:42:37Z Only to Americans ;) But I agree explaining this is good.
I hadn't noticed that there are no seconds in the first
I've added text that renders the dates in ISO format. frizatch commented on 2025-07-18T23:06:33Z Ha ha, it wasn't the year to month to day that was confusing... it was when it kept going into seconds. I think what you added is good! Simply pre-pend a statement like, "For a more readable output of the temporal range we can run:" |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:04Z Typo: missing a "k" now > know |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:04Z Suggestion: add some text describing the (illegible) legend for the MODIS granules ... at least a mention that the light blue color represents clouds:
green > yellow > oranges > reds = percentage of snow in cell 0 to 100 magenta = night blue = inland water dark blue = ocean light blue = cloud grey = no decision pink = missing data dark red = detector saturated
andypbarrett commented on 2025-07-17T22:48:00Z Updated the text to explain colors and also note that a larger version of the thumbnail (handprint) is available by clicking on the image.
|
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:05Z Throw in a legend? Or to keep the code simple, verbally describe a color ramp based on snow thickness will appear by default? andypbarrett commented on 2025-07-17T22:59:11Z Done |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:06Z Typo: direcory needs a "t" |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:06Z Add warning that not all GDALs have the same drivers and one should check (mine is fine with .h5, but not .hdf) andypbarrett commented on 2025-07-17T23:01:30Z Shakes fist at the god of gdal |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:07Z I think being simple and staying with a GPR data clip is good. But this brings up the question about having the SNOTEL site in this tutorial at all. It does get plotted later, but is it just a distraction? Nothing is DONE with it. If it is left in as a nice example of field data, more orientation about SNOTEL could/should be added. andypbarrett commented on 2025-07-17T23:23:18Z Agreed. Removed it. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:08Z COLORS, no, unfortunately I don’t think it works. We are relying on the SnowEx data to “disappear” if it actually matches the ASO data, which is odd. There is the validation by contrast (flip one color ramp). I am remiss to try anything with transparency because that changes the colors you’d see in the legend ramp. Maybe need two legends side by side, either with an inverted ramp, or a different color scheme. The ASO data would be overlain, but you could mentally interpolate from the ASO that is visible. These high-density point data sets overlay themselves all the time anyway. andypbarrett commented on 2025-07-17T23:22:57Z Updated to use contrasting color ramps. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:08Z Add a quick sentence about why interpolation could be preferred here given the resolution as opposed to the extract nearest value as is done for MODIS later? A person going through this might expect a straight overlay extraction and wonder why .interp is used. andypbarrett commented on 2025-07-17T23:35:45Z Good point. I've updated to say that because the ASO data is high resolution (3 m) interpolation is a reasonable option but suggest nearest-neighbour as an alternative. |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-14T23:26:09Z I am not sure about how the static version of the rendered notebook will appear to users, but in some cases, the file icon and the database icons do not appear and the text is simply formatted in ASCII instead of the grey/white tables in an active notebook, so the "clicking on the file icon" verbiage can be confusing. Maybe "clicking on the file icon that appears in an active notebook" works if this a case worth worrying about? andypbarrett commented on 2025-07-17T23:38:33Z Yes. This is an issue with how notebooks are rendered in GH. Ideally, we would have a web-rendered version of all the tutorials and a way to download the notebooks. I'm not going to worry about it. frizatch commented on 2025-07-18T23:49:15Z Sounds good! |
Changed to exporting snowexp_gpr instead View entire conversation on ReviewNB |
I have dealt with almost all of the comments. Please take another look. I think there are only two outstanding comments to be addressed at this point.
I also need to see if we can add an extension to allow variables to be rendered in MD cells. |
I did some further investigation on the UTM zone snafu. According to [Wikipedia], lettered latitudinal bands are not part of the UTM specification.
The UTM 12S CRS registered with EPSG is for the southern hemisphere. I also checked the conversion of geographic to projected coordinates for the three GPR files. The week 1 file x,y coordinates in the file are within 0.01 m of the transformed geographic coordinates. However, for the week 2 file, there is a big mismatch because the projected (x,y) coordinates are a mixture of UTM 12N amd UTM 13N. For week 3, there is a maximum difference of -1.75 m between the y values in the file and transformed geographic coordinates. The differences for the x-coordinates are smaller (-0.04 to -0.02 m). Although these differences are not big, with the exception of the week 2 data, given GPS accuracy (I'm assuming worst case ~10 m), it becomes a time sink if someone compares geographic and projected coordinates in the file. There is also another issue with week 2, there are duplicate points. Not sure about the other files. |
I have an inquiry in with tech writers / USO. View entire conversation on ReviewNB |
Ha ha, it wasn't the year to month to day that was confusing... it was when it kept going into seconds. I think what you added is good! Simply pre-pend a statement like, "For a more readable output of the temporal range we can run:" View entire conversation on ReviewNB |
Sounds good! View entire conversation on ReviewNB |
Here is the output: /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/constructive.py:181: RuntimeWarning: invalid value encountered in buffer return lib.buffer( /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/cartopy/io/__init__.py:241: DownloadWarning: Downloading: https://naturalearth.s3.amazonaws.com/110m_physical/ne_110m_coastline.zip warnings.warn(f'Downloading: {url}', DownloadWarning) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:688: RuntimeWarning: invalid value encountered in covers return lib.covers(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:730: RuntimeWarning: invalid value encountered in disjoint return lib.disjoint(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:730: RuntimeWarning: invalid value encountered in disjoint return lib.disjoint(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:688: RuntimeWarning: invalid value encountered in covers return lib.covers(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:730: RuntimeWarning: invalid value encountered in disjoint return lib.disjoint(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:688: RuntimeWarning: invalid value encountered in covers return lib.covers(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/constructive.py:181: RuntimeWarning: invalid value encountered in buffer return lib.buffer( /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:730: RuntimeWarning: invalid value encountered in disjoint return lib.disjoint(a, b, **kwargs) /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/shapely/predicates.py:688: RuntimeWarning: invalid value encountered in covers return lib.covers(a, b, **kwargs) View entire conversation on ReviewNB |
But again, everything still works, just with many warnings View entire conversation on ReviewNB |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-18T23:56:45Z I am actually getting a warning here as well: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81. import pkg_resources andypbarrett commented on 2025-07-22T18:28:30Z I propose surpressing warnings for now.
import warnings warnings.filterwarnings("default", category=DeprecationWarning, module=user_ns.get("__name__")) frizatch commented on 2025-07-22T19:26:09Z Agree! andypbarrett commented on 2025-07-22T20:21:19Z @frizatch Can you run the following in a jupyter cell # For checking versions # Can be deleted before publication # Andy's environment has: # earthaccess: 0.9.0 # pandas: 2.2.0 # geopandas: 0.14.4 # xarray: 2024.7.0 # rioxarray: 0.15.0 # matplotlib: 3.9.4 # cartopy: 0.23.0 # numpy: 1.23.5 import sys import cartopy print(f"Platform: {sys.platform}") print(f"Python (sys): {sys.version}") for pkg in [earthaccess, pd, gpd, xr, rioxarray, mpl, cartopy, np]: print(f"{pkg.__name__}: {pkg.__version__}") frizatch commented on 2025-07-22T21:53:05Z Hmmm, okay. I manually pulled a environment.yml file from the SnowEx_ASO_MODIS_Snow folder on the main branch (I HAD been in an environment that I created from the environment.yml file found at NSIDC-Data-Tutorials/binder - I thought it was built for all the tutorials), added the rioxarray dependency, then ran what you suggested above (with importing the appropriate libraries) and got (with no warnings!):
Platform: darwin Python (sys): 3.9.23 | packaged by conda-forge | (main, Jun 4 2025, 18:02:02) [Clang 18.1.8 ] earthaccess: 0.12.0 pandas: 2.3.1 geopandas: 1.0.1 xarray: 2024.7.0 rioxarray: 0.15.0 matplotlib: 3.9.4 cartopy: 0.23.0 numpy: 2.0.2 |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-18T23:56:45Z Nice! |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-18T23:56:46Z As an interesting benefit for me rebuilding my computer today, I discovered you can get through the notebook up to here without a .netrc file, but without one, this is the first codeblock that fails - which makes sense because this is where the actual access starts.
Let's add a sentence about needing this earthdata login at the beginning with a link to: https://nsidc.org/data/user-resources/help-center/creating-netrc-file-earthdata-login andypbarrett commented on 2025-07-22T20:26:45Z I need to add a |
View / edit / reply to this conversation on ReviewNB frizatch commented on 2025-07-18T23:56:46Z Add another # to the first line so it remains a comment if a user Ctrl/'s the whole block? |
Due to the website unification project, it was decided that the doi urls should stay, so this is good. View entire conversation on ReviewNB |
View / edit / reply to this conversation on ReviewNB saberbrasher commented on 2025-07-22T18:04:34Z Line #17. import matplotlib.pyplot as plt This is a great line! andypbarrett commented on 2025-07-22T18:13:59Z Thank you. |
x2. View entire conversation on ReviewNB |
Thank you. View entire conversation on ReviewNB |
I propose surpressing warnings for now.
import warnings warnings.filterwarnings("default", category=DeprecationWarning, module=user_ns.get("__name__")) View entire conversation on ReviewNB |
Agree! View entire conversation on ReviewNB |
This is temporarily dealt with by suppressing warnings View entire conversation on ReviewNB |
@frizatch Can you run the following in a jupyter cell # For checking versions # Can be deleted before publication # Andy's environment has: # earthaccess: 0.9.0 # pandas: 2.2.0 # geopandas: 0.14.4 # xarray: 2024.7.0 # rioxarray: 0.15.0 # matplotlib: 3.9.4 # cartopy: 0.23.0 # numpy: 1.23.5 import sys import cartopy print(f"Platform: {sys.platform}") print(f"Python (sys): {sys.version}") for pkg in [earthaccess, pd, gpd, xr, rioxarray, mpl, cartopy, np]: print(f"{pkg.__name__}: {pkg.__version__}") View entire conversation on ReviewNB |
I need to add a View entire conversation on ReviewNB |
I added in import for earthaccess, pandas, geopandas, xarray, rioxarray, matplotlib, and numpy so the /Users/dfritz/mamba/envs/nsidc-tutorials/lib/python3.9/site-packages/earthaccess/formatters.py:4: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81. import pkg_resources Platform: darwin Python (sys): 3.9.18 | packaged by conda-forge | (main, Aug 30 2023, 03:53:08) [Clang 15.0.7 ] earthaccess: 0.9.0 pandas: 2.3.1 geopandas: 0.14.4 xarray: 2024.7.0 rioxarray: 0.15.0 matplotlib: 3.9.4 cartopy: 0.23.0 numpy: 1.23.5 View entire conversation on ReviewNB |
I thought I had added a response to this comment but don't see it.
The UTM system only has longitude bands, and northern and southern hemisphere zones. The latitudinal bands are part of the Military Grid Reference System (see https://en.wikipedia.org/wiki/Universal_Transverse_Mercator_coordinate_system#Latitude_bands). This corresponds with the CRS for UTM 12 N registered with EPSG.
The
This is a dataset problem and something that the tech writers and dataset producers need to correct. I think for now, we have to put a note here and in the User Guide to point out these issues. IMO the map you link to exacerbates the "wrongness" of the data files. View entire conversation on ReviewNB |
Hmmm, okay. I manually pulled a environment.yml file from the SnowEx_ASO_MODIS_Snow folder on the main branch (I HAD been in an environment that I created from the environment.yml file found at NSIDC-Data-Tutorials/binder - I thought it was built for all the tutorials), added the rioxarray dependency, then ran what you suggested above (with importing the appropriate libraries) and got (with no warnings!):
Platform: darwin Python (sys): 3.9.23 | packaged by conda-forge | (main, Jun 4 2025, 18:02:02) [Clang 18.1.8 ] earthaccess: 0.12.0 pandas: 2.3.1 geopandas: 1.0.1 xarray: 2024.7.0 rioxarray: 0.15.0 matplotlib: 3.9.4 cartopy: 0.23.0 numpy: 2.0.2 View entire conversation on ReviewNB |
@@ -0,0 +1,1382 @@ | |||
{ |
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.
This call is still hitting ecs:
ClientResponseError: 404, message='Not Found', url='https://n5eil01u.ecs.nsidc.org/DP1/SNOWEX/SNEX17_GPR.002/2017.02.08/SnowEx17_GPR_Version2_Week1.csv'
(I ran this notebook with an adjusted environment.yml file from the tutorial folder)
Reply via ReviewNB
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.
Weirdly, with the environment.yml from the binder folder, it DID work, but the later call for downloading ASO_3M_SD_USCOGM_20170208.tif
hit ecs again.
Side note: the binder yml allows for the granules-returned messages, whereas the yml in the folder does not provide an environment where these work.
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.
Setting cloud_hosted=True
fixes the issue ... the issue likely being caused by my extra persmissions in being able to see ecs data. This statement is present for MODIS data, so I suggest adding it for SnowEx and ASO as well for consistency.
Updated tutorial to use
earthaccess
for search and access.Updated to use
geopandas
andxarray
, andrioxarray
for reading data, reprojection and data extraction.Refactored workflow to avoid use of bespoke utilities.