netcdf-java
netcdf-java copied to clipboard
Unidata
[5.x]: Adding the ability to save geotiffs with a palette
Description of Changes
Add support to save out a geotiff with supplied colortable
PR Checklist
- [x] Indicate the version associated with this PR in the Title (e.g. "[5.x]: This is my PR title")
- [x] Link to any issues that the PR addresses
- [ ] Add labels, especially if the PR should be ported to other versions (these labels start with "port: ")
- [x] Open as a draft PR until ready for review
- [x] Make sure GitHub tests pass
- [x] Mark PR as "Ready for Review"
This is my first significant chunk of Java code, so don't assume I did any of this correctly.
The reason for this submission and its design is that I plan to also make a submission to TDS's WCS service to add a new Geotiff_Palette format that would then respect a dataset's "flag_values" and "flag_colors" attributes, much like how ncWMS does right now. We could also respect supplied styling options this way, too.
And, this is partly in response to https://github.com/Unidata/netcdf-java/issues/1052
I am just about done with a set of revisions that I think makes things more sane, but I needed to write a coerceInt() and a coerceByte() private methods to coerce an arbitrary Array into ArrayInt or ArrayByte copies, respectively. I feel like that is some sort of NiH syndrome. Is there already a "correct" way to do what in numpy would be like an a.astype(int)?
Yes in the sense that it is a feature for a client of mine, so I've been maintaining it for them. There is a bug of sorts, but I think it is actually pre-existing. I would appreciate feedback on what I have so far and if there is anything else you all would like added to it.
On Thu, Apr 11, 2024 at 2:53 PM Tara Drwenski @.***> wrote:
Hi @WeatherGod https://github.com/WeatherGod, are you still working on this PR?
— Reply to this email directly, view it on GitHub https://github.com/Unidata/netcdf-java/pull/1066#issuecomment-2050317013, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACHF6CRDFEFHFNX3B5J32LY43L3DAVCNFSM57JF5HWKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBVGAZTCNZQGEZQ . You are receiving this because you were mentioned.Message ID: @.***>
Okay, thanks! Will try to have a look at your comments about the possible bug. To see the style issues you can run ./gradlew spotlessCheck or to automatically fix them you can run ./gradlew spotlessApply
Crazy thought,, but what if we allowed color tables to work with greyscale==true? Right now, as coded, greyscale mode scales the given data to the range of 1-255 and encodes missing data as 0, effectively unsigned bytes, while the "colortable" mode doesn't do any scaling at all, and only works if the data type is unsigned bytes. But, it is getting tedious to document and code checks that the colortable can only be used when greyscale is false. Why not allow color tables to be used for greyscale mode?
It might be easier to explain the various features available as "greyscale normalizes to UBYTE", "color table works only for UBYTEs", and "Non-greyscale FLOATs encodes missing data as data minimum minus one".
Thoughts?
Now that I've actually implemented that idea, I really like it. I'm running with it. It also addresses several other questions I had in my mind about possible use-cases and whether or not the setColorTable() was an anti-pattern.
Test failures seem to be unrelated?
Hi @WeatherGod, I think I know the issue there, should have a fix shortly!
Is there anyplace where I should add some narrative docs explaining the changes to the geotiff module?
@WeatherGod, sorry for the delay getting back to this. We don't have any documentation that I can find about geotiffs. So I think the best spot for documentation is in the javadocs for these classes. We can also add a line to our next release's notes like "Support for saving Geotiffs with a palette", that will hopefully alert users to have a look at your changes.
Ok, that makes sense. Do you want something more than what I've written so far?
Failing test is a flaky test unrelated to your changes and we are fixing it in a separate PR
Thank you very much! Question: does the CI/CD for the tds repo use the latest release of netcdf-java or the development branch? I have what is essentially the second half of this feature which allows users to download geotiffs with the colormap from thredds WCS service. It isn't as fully fleshed out as this one was, but it would be nice to at least get a draft PR up.
Thank you very much! Question: does the CI/CD for the tds repo use the latest release of netcdf-java or the development branch?
It uses the latest netcdf-java snapshot, which is functionally the development branch.
