bcdev / snap-idepix

Collection of IdePix pixel classification modules for various sensors
GNU General Public License v3.0
4 stars 1 forks source link

Facilitate retrieval of CTP even if cloud shadow is not computed #33

Closed heptaflar closed 3 years ago

heptaflar commented 3 years ago

This pull request would enable the retrieval of CTP on request, even if cloud shadow is not requested. This makes sense in cases where one needs CTP in subsequent processing steps but does not need cloud shadow or is going to compute cloud shadow by different means, which need CTP as input.

dolaf commented 3 years ago

looks ok for me

marpet commented 3 years ago

In the @Parameter annotation alias="computeCtp" could be added. Then outputCtp will still work, but computeCtp has precedence.

marpet commented 3 years ago

Is this applicable to other sensors besides OLCI? Just to harmonise things.

heptaflar commented 3 years ago

I agree with the first and the last point Tonio raised. Renaming the parameter to "computeCtp" while introducing an alias "outputCtp" ensures compatibility with previous versions. Thanks Marco for pointing this out.

I am not sure about Tonio's second point, however. For me, cloud shadow and CTP are separate concerns. I prefer the present behaviour, i.e., CTP is only put out, if requested explicitly. Anyway, Tonio's suggestion would work for me, too.

I modified the original pull request to address point 1 and 3 on Tonio's list.

TonioF commented 3 years ago

I am not sure about Tonio's second point, however. For me, cloud shadow and CTP are separate concerns. I prefer the present behaviour, i.e., CTP is only put out, if requested explicitly. Anyway, Tonio's suggestion would work for me, too.

Any more thoughts on this? Then we can have a majority vote. I'd be fine with accepting now.

marpet commented 3 years ago

I would vote for Ralfs solution and leaving CTP and cloud shadow independent.

dolaf commented 3 years ago

I would also vote for keeping them separate. Also, the user should still have the choice not to compute CTP if not needed. Every bit of performance is useful :-)

Olaf

Am 29.01.2021 um 11:45 schrieb Marco Peters:

I would vote for Ralfs solution and leaving CTP and cloud shadow independent.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bcdev/snap-idepix/pull/33#issuecomment-769728580, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUGBR3IJSCUOLVB4P26QTS4KGVDANCNFSM4WXMDFVA.

--


Dr. Olaf Danne Brockmann Consult GmbH phone: +49 (0)40 696389 313 email: olaf.danne@brockmann-consult.de skype-id: olaf.danne.bc


Brockmann Consult GmbH Chrysanderstr. 1 D-21029 Hamburg, Germany Amtsgericht Hamburg HRB 157689 Geschäftsführer Dr. Carsten Brockmann Web: www.brockmann-consult.de Twitter: @BrockmannCon


heptaflar commented 3 years ago

Accepted by me. Who merges?

marpet commented 3 years ago

Probably Olaf, when he finds the time to do it

marpet commented 3 years ago

Not needed anymore. CTP calculation is not the purpose of IdePix. The output is only included for debugging purpose.