Skip to content
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 an optional kwarg to CloudCompositor for RGBA output #2788

Closed
wants to merge 3 commits into from

Conversation

yukaribbba
Copy link
Contributor

@yukaribbba yukaribbba commented Apr 22, 2024

Usually CloudCompositor produces an LA mode image. But sometimes you may want RGBA result. So this PR will add "mode" kwarg just like MaskingCompositor.

  • Tests added
  • Fully documented

Copy link

codecov bot commented Apr 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.94%. Comparing base (87d072d) to head (1c242a0).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2788   +/-   ##
=======================================
  Coverage   95.94%   95.94%           
=======================================
  Files         379      379           
  Lines       53693    53713   +20     
=======================================
+ Hits        51515    51535   +20     
  Misses       2178     2178           
Flag Coverage Δ
behaviourtests 4.09% <4.76%> (+<0.01%) ⬆️
unittests 96.04% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coveralls
Copy link

coveralls commented Apr 22, 2024

Pull Request Test Coverage Report for Build 8785324481

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 8781051256: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

@yukaribbba yukaribbba marked this pull request as ready for review April 22, 2024 14:22
@djhoese
Copy link
Member

djhoese commented Apr 22, 2024

Could you explain more why/when this is needed? Normally writers or other compositors are supposed to handle these cases for you.

@yukaribbba
Copy link
Contributor Author

My fault. Didn't realize that. Closing now

@yukaribbba yukaribbba closed this Apr 23, 2024
@yukaribbba yukaribbba deleted the add_rgba_cloudcompositor branch April 23, 2024 01:12
@djhoese
Copy link
Member

djhoese commented Apr 23, 2024

If you have a use case where this isn't true let me know. There is very likely a case where things won't work, I just didn't know what that situation would be.

@yukaribbba
Copy link
Contributor Author

Well some image viewing software can't display LA images correctly, such as Photo Viewer in Windows. That's why I start this. But your words remind me that I can build something with GenericCompositor like

    compositor: !!python/name:satpy.composites.GenericCompositor
    prerequisites:
      - name: C14
      - name: C14
      - compositor: !!python/name:satpy.composites.CloudCompositor
        prerequisites:
          - name: C14

But there are still some errors I'm trying to figure out.

@yukaribbba
Copy link
Contributor Author

yukaribbba commented Apr 23, 2024

So GenericCompositor isn't helpful since CloudCompositor will add a "band" dimension which is not present in the first two. Therefore triggers the "incompatible areas". So I turn to MaskingCompositor and BackgroundCompositor.

  masking:
    compositor: !!python/name:satpy.composites.MaskingCompositor
    prerequisites:
      - name: C14
      - name: C14
    conditions:
      - method: less_equal
        value: 0
        transparency: 100
      - method: greater
        value: 0
        transparency: 100
      - method: isnan
        transparency: 100
    mode: RGBA
    standard_name: no_enhancement_spec

  ir_cloud_day_default_2000:
    compositor: !!python/name:satpy.composites.BackgroundCompositor
    prerequisites:
      - name: masking
      - compositor: !!python/name:satpy.composites.CloudCompositor
        prerequisites:
          - name: C14
    standard_name: ir_cloud_day_default

The logic is: lay a completely transparent RGBA image upon the LA image(day cloud) then you get the RGBA result because BackgroundCompositor will do that add_bands job. It's kinda magical but anyway I got what I want and no changes needed.

@djhoese
Copy link
Member

djhoese commented Apr 23, 2024

Why is C14 being used here? Is the final result you want just a geotiff with the cloud compositor output as RGBA? And you need that so more image viewers will show the image correctly?

@yukaribbba
Copy link
Contributor Author

Yes I want a geotiff in RGBA mode so that my image viewer can show it properly. C14 is used just for convenience since cloudcompositor will also use that band. Actually it can be any bands since all I need from that is just a transparent image . I'm not sure if other image viewers have such problems but in my Windows pc that is certain. Could just be a rare case, who knows...

@djhoese
Copy link
Member

djhoese commented Apr 23, 2024

I think this is a good feature, but should maybe not be added to just the CloudCompositor. I wonder if there is a way to do this with enhancements. @pnuu @simonrp84 or @mraspaud any ideas how to take the output of a composite that is LA and produce an RGBA without modifying Satpy or trollimage?

@pnuu
Copy link
Member

pnuu commented Apr 23, 2024

The first option that comes to mind is to create a composite that has C14 on all 3 channels and then use that with CloudCompositor.

@yukaribbba
Copy link
Contributor Author

@pnuu Unfortunately CloudCompositor can only accept a single band otherwise it'll end it up like this:

<xarray.DataArray 'bands' (bands: 6)> Size: 24B
array(['R', 'G', 'B', 'R', 'G', 'B'], dtype='<U1')
Coordinates:
    crs      object 8B PROJCRS["unknown",BASEGEOGCRS["unknown",DATUM["Unknown...
  * bands    (bands) <U1 24B 'R' 'G' 'B' 'R' 'G' 'B'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants