Pass topLevelXML path by TOA file

[supermarkion]

Hello,

Nice to meet you and how are you.

Thanks for your excellent work on this library. This library helps us a lot.

I found since v0.5.6 (https://github.com/ubarsc/python-fmask/compare/pythonfmask-0.5.5...pythonfmask-0.5.6), this library only supports Sentinel-2 run with safedir or granuledir because of the way we access offset values from MTD_MSIL1C.xml file.

I added this PR, which can pass the metadata XML path via the TOA file (by using gdal_edit CLI). The added section looks like:

                                                          use `sen2meta.Sen2ZipfileMeta(XML_path)` to get `topMeta`
                                                           /
                                                        yes
                                                        /
             Is there the XML_path value in TOA file?
                                                       \
                                                       no /  XML file not there
                                                          \
                                                          throw FmaskFileError

Please let me know how do you think this change.

Cheers

Sai

[neilflood]

I don't think this is a good solution. It involves some very specific assumptions about how you have modified the TOA stack, to add a specific metadata item. I would rather not code that sort of thing into the generic package.

The right approach depends on exactly how you are using the package. In our own group, we do not actually use the main script as given. We stack the bands together ourselves, and this file is the basic unit for our work. We have our own main script which understands how we have organised the files, and does things appropriate for that.

It seems like you are also reorganising the files, but then wanting to use the standard main script, which only knows about the standard ESA file arrangement. Is that right?

I think that if you want to be able to work with re-arranged files, you should write your own main script which knows about this. It won't be difficult, mostly it should look like the mainRoutine() function in sentinel2Stacked.py. Where that reads the top meta with the readTopLevelMeta() function, you can instead just use something like

topMeta = sen2meta.Sen2ZipfileMeta(xmlfilename=topLevelXML)

since you already know where the XML file is.

Have I understood your situation correctly?

[supermarkion]

Hello, @neilflood, thanks a lot for your response. Appreciate your detailed clarification (e.g. how your team use this library).

In our group, we are using the main script to run the pipeline, specially the run with TOA case. We are using the gdalbuildvrt to build a vrt file as TOA file with JP2 files from Sentinel-2 zip file. Therefore, when we change to the new version (v.0.5.6), we found we have to unzip all the zip files because of the way to access metadata XML in main script.

Of course, as you said, we can change/build new main script to walkarond this. Actually I am using gdal_edit and gdal_translate to apply the offset values in vrt file to make our pipeline running again if you are intertesed how we handle it.

The main reason why I submit this PR aims get the Python-FMask Sentinel-2 run with TOA feature back to community. As you known, right now the S2 run with TOA feature labels as obsolete. May I ask if I want to make this feature back, does this PR good enough or maybe I should change it in another way?

Thanks a again for your help.

Cheers

Sai

[neilflood]

No, this PR is not good enough, and will not be merged. I am not going to take on the task of maintaining compatibility with whatever arrangement of files you happen to have decided upon. People who are not using the standard ESA file structures should maintain their own code to manage this.

Furthermore, if you are applying the offsets yourself (really ?), then this is even more reason to manage your own code. This PR would give incorrect answers, because the offsets would end up being applied twice, once by you, and once by my code.

The --toa option was part of our initial test framework, and we kept it going even once the --safedir option was added. However, since ESA added the radiometric offsets, requiring the use of the top-level XML file, it is not really practical to keep it going. I don't expect to put in any time on maintaining it in future.

[supermarkion]

No problem, Neil. Super happy to get your idea and feedback about how to use this library.

Thanks for your help and feel free to close my PR and Issue.