-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add NSIDCbin (National Snow and Ice Data Centre Sea Ice Concentrations) raster driver #6183
Conversation
for the cppcheck warnings about unused private members (https://github.com/OSGeo/gdal/runs/7781642042?check_suite_focus=true), you can use patterns like
|
thank you @rouault ! very much appreciated feedback |
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.
The crash is completely unrelated:
The issue is that you dereference pbSuccess without checking it for != nullptr before (callers may pass a nullptr pbSuccess pointer) |
Thanks @rouault your guidance is very very good, much appreciated. For now I'm leaving as Byte with full range of values (special values > 250). I really want this for use with the warper I will likely have to do what was suggested on gdal-dev:
https://lists.osgeo.org/pipermail/gdal-dev/2022-August/056144.html That post and the feedback really cleared up for me how things work. 🙏 |
https://github.com/OSGeo/gdal/runs/7857751581?check_suite_focus=true:
|
We now have linting rules of autotest python files
And to make it run then on modified files: |
excellent thanks again 🙏 |
this format has been replaced by a (decent afaics) NetCDF upgrade: https://nsidc.org/data/nsidc-0051/versions/2 I'd still like this legacy driver to be included, and I'm prepared to continue maintenance - we use this extensively and it will be helpful for me to have it as alternative pathway during various upgrades going on - but I can understand if this changes the decision on inclusion. |
if there are significant amount of past data using the legacy format and likely not being converted and published into the new netCDF format, that probably justifies keeping the driver |
Thanks Even, I think they'll all be covered by the new form - will take a few days for our sync to catch up but I'll check a few things and report back more fully before I shut this down. |
@mdsumner any update ? |
still doing checks and things I'd like to compare - there are particular netcdf oddities - for example they seem to store days without data as a shell netcdf, and there's a few of these things to investigate - but overall it would be very handy to have this for ongoing work, it will be some time until we can confidently move completely to the new files, and there will be many copies of these older version files around, so - I'd still like it to be included in a future GDAL release. 🙏 |
The main problem I foresee is emails from folks telling us there's a netcdf version now 😄 - I would like this to go in, but the fact is I can do my work in a dev fork locally so I'm happy if you decide not to include it. I'm committed to maintaining it, it's a good lever for me to learn the next level of RasterBand customization. 🙏 |
Another pitch, I really want to have it available for ease of comparing virtual mdim approaches - is it really better with netcdf ... etc - and what does Zarr (or similar) creation look like for identical data from different formats, an active area of investigation for us coming up. |
ok, makes sense |
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
I struggled with the rebase, feel like I haven't done it right, I was incrementally merging in master ... Not given up I just can't finish with the checks rn 🙏 |
your PR is in a wrong state with hundreds of files appearing modified. You probably git merge instead of git rebase. |
Even a git merge (with the right branch) wouldn't indicate so many files on Github. After the merge, most files should be identical. Did you pull the latest HEAD of master from this repo? Did you perhaps rebase on an outdated local master brauch? |
yes sorry, I really messed it up, I never use rebase and don't really understand it I'll start again |
Supported by GDAL for read access. This format is a raw binary format for the Nimbus-7 SMMR and DMSP SSM/I-SSMIS Passive Microwave Data sea ice concentrations. There are daily and monthly maps in the north and south hemispheres supported by this driver.
A simple test and example file is included.
Original discussion query was made here: https://lists.osgeo.org/pipermail/gdal-dev/2015-July/042280.html
Tasklist
extend the tests to include actual values and expected first non-missing value etc
resolve issue of which auth - change in time, no use non-deprecated: https://nsidc.org/data/user-resources/help-center/guide-nsidcs-polar-stereographic-projection
Add test case(s)
Add documentation (include examples to download file/s, hit ftp with vsi:// etc)
Review
Adjust for comments
All CI builds and checks have passed
standardize variable names according to project norms
optional extras, future work