Opened 6 years ago

Closed 6 years ago

#2407 closed defect (fixed)

Rename i.tasscap to i.landsat.tasscap?

Reported by: jbrauner Owned by: grass-dev@…
Priority: blocker Milestone: 7.0.0
Component: Imagery Version: svn-releasebranch70
Keywords: i.tasscap, landsat Cc:
CPU: All Platform: All

Description

Hi GRASS devs,

the module description for i.tasscap states "Performs Tasseled Cap (Kauth Thomas) transformation for LANDSAT-TM data".

If the module only works for LANDSAT data, I'd suggest to rename it to i.landsat.tasscap. If the module also works for other satellite imagery the module description should be broadened a little bit.

Sorry, I'm not a remote sensing expert myself.

As suggested by Martin L. last week I marked it as blocker.

Cheers,

Johannes

Change History (10)

comment:1 Changed 6 years ago by neteler

Good point but in my view it would be better to add more sensors, e.g. Quickbird, CBERS etc. In case we get those other tasseled cap coefficients (I see that some have been published online) we could add a sensor=xxx parameter.

comment:3 in reply to:  1 ; Changed 6 years ago by martinl

Resolution: fixed
Status: newclosed

Replying to neteler:

Good point but in my view it would be better to add more sensors, e.g. Quickbird, CBERS etc. In case we get those other tasseled cap coefficients (I see that some have been published online) we could add a sensor=xxx parameter.

So close the ticket as wontfix?

comment:4 in reply to:  3 ; Changed 6 years ago by jbrauner

Replying to martinl:

Replying to neteler:

Good point but in my view it would be better to add more sensors, e.g. Quickbird, CBERS etc. In case we get those other tasseled cap coefficients (I see that some have been published online) we could add a sensor=xxx parameter.

So close the ticket as wontfix?

Absolutely fine by me. I've no strong feelings about leaving it as is (as I even cannot help to improve it to include new sensors).

Side note: I'm not sure if a new (even blocking?) ticket is required, asking for these new parameters like 'sensor' to be foreseen in the future - I'm just wondering because of the upcoming RC. I'm also guessing that a number of other parameters could change as well. Certainly not my decision to make :)

Cheers,

Johannes

comment:5 in reply to:  4 ; Changed 6 years ago by wenzeslaus

Replying to jbrauner:

Side note: I'm not sure if a new (even blocking?) ticket is required, asking for these new parameters like 'sensor' to be foreseen in the future - I'm just wondering because of the upcoming RC. I'm also guessing that a number of other parameters could change as well. Certainly not my decision to make :)

I'm not following this issue but in general blocker is required only if the API is changed in backwards incompatible manner. So, if you are adding parameter but it has a default which gives the equivalent result as the previous version it is OK.

comment:6 in reply to:  5 Changed 6 years ago by jbrauner

Replying to wenzeslaus:

Replying to jbrauner:

Side note: I'm not sure if a new (even blocking?) ticket is required, asking for these new parameters like 'sensor' to be foreseen in the future - I'm just wondering because of the upcoming RC. I'm also guessing that a number of other parameters could change as well. Certainly not my decision to make :)

I'm not following this issue but in general blocker is required only if the API is changed in backwards incompatible manner. So, if you are adding parameter but it has a default which gives the equivalent result as the previous version it is OK.

Oh, I see, thanks a lot for the clarification.

Cheers,

Johannes

comment:7 in reply to:  5 ; Changed 6 years ago by neteler

Resolution: fixed
Status: closedreopened

Replying to wenzeslaus:

I'm not following this issue but in general blocker is required only if the API is changed in backwards incompatible manner.

Yes: In this case the actual flags need to be turned into values for the new sensor= parameter. (see http://grass.osgeo.org/grass70/manuals/i.tasscap.html):

sensor=landsat4,landsat5,landsat7,landsat8,quickbird,...

comment:8 in reply to:  7 ; Changed 6 years ago by wenzeslaus

Replying to neteler:

Replying to wenzeslaus:

I'm not following this issue but in general blocker is required only if the API is changed in backwards incompatible manner.

Yes: In this case the actual flags need to be turned into values for the new sensor= parameter. (see http://grass.osgeo.org/grass70/manuals/i.tasscap.html):

sensor=landsat4,landsat5,landsat7,landsat8,quickbird,...

The new version committed in r61920 to trunk has satellite option instead of flags and one input option instead of several band* options.

Several naming changes done with MarkusN in order to be more precise about satellite/sensor. Feedback welcome, especially the manual page should be reviewed. I tested that but more testing would be helpful too.

Now it is possible to add new satellites/sensors easily without breaking API.

This ticket can be closed once the change is backported to 7.0.

comment:9 in reply to:  8 Changed 6 years ago by jbrauner

Replying to wenzeslaus:

Replying to neteler:

Replying to wenzeslaus:

I'm not following this issue but in general blocker is required only if the API is changed in backwards incompatible manner.

Yes: In this case the actual flags need to be turned into values for the new sensor= parameter. (see http://grass.osgeo.org/grass70/manuals/i.tasscap.html):

sensor=landsat4,landsat5,landsat7,landsat8,quickbird,...

The new version committed in r61920 to trunk has satellite option instead of flags and one input option instead of several band* options.

Several naming changes done with MarkusN in order to be more precise about satellite/sensor. Feedback welcome, especially the manual page should be reviewed. I tested that but more testing would be helpful too.

Seems to work as expected (however I cannot check whether the calculations are semantically correct), manual seems fine as well. Great job! Once again I'm fairly impressed by you devs.

Now it is possible to add new satellites/sensors easily without breaking API.

This ticket can be closed once the change is backported to 7.0.

Cheers,

Johannes

comment:10 Changed 6 years ago by neteler

Resolution: fixed
Status: reopenedclosed

Backported in r62125 and r62127. Closing.

Note: See TracTickets for help on using tickets.