-
Notifications
You must be signed in to change notification settings - Fork 69
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
[5.x]: Adding the ability to save geotiffs with a palette #1066
base: maint-5.x
Are you sure you want to change the base?
Conversation
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 #1052 |
I am just about done with a set of revisions that I think makes things more sane, but I needed to write a |
cdm/misc/src/test/java/ucar/nc2/geotiff/TestGeoTiffPalette.java
Outdated
Show resolved
Hide resolved
cdm/misc/src/test/java/ucar/nc2/geotiff/TestGeoTiffPalette.java
Outdated
Show resolved
Hide resolved
ee775e7
to
98056eb
Compare
98056eb
to
f8f4a6d
Compare
Hi @WeatherGod, are you still working on this PR? |
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
<#1066 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACHF6CRDFEFHFNX3B5J32LY43L3DAVCNFSM57JF5HWKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBVGAZTCNZQGEZQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
f8f4a6d
to
ea24e9f
Compare
Okay, thanks! Will try to have a look at your comments about the possible bug. To see the style issues you can run |
181d07b
to
b5a17bb
Compare
* Fixing some data-handling issues * Make the greyscale mode completely separate from the colortable handling * Added the ability to specify the output data type
* Particularly noticeable for sample min/max metadata
* Expand unit tests for geotiff writing * Reduce code duplication
2008dd2
to
617703d
Compare
Crazy thought,, but what if we allowed color tables to work with 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. |
* Make it possible to use colortable regardless of greyscale mode, so long as it is UBYTE * Fix some of the coercion code * Added more tests * Improved documentation and input checking
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? |
Description of Changes
Add support to save out a geotiff with supplied colortable
PR Checklist
(e.g. "[5.x]: This is my PR title")
(these labels start with "port: ")
until ready for review