EnMAP-Box / enmap-box

EnMAP-Box source code repository. See https://enmap-box.readthedocs.io for documentation
GNU General Public License v3.0
35 stars 16 forks source link

Interactive setting of bad bands in the Raster Layer Styling/Gray #31

Closed aloboa closed 1 year ago

aloboa commented 2 years ago

Show the info on whether a given band has been set as Bad Band in the Raster Layer Styling/Gray, and let the user tick/untick that band as bad band.

janzandr commented 2 years ago

(@jakimowb , correct me if I'm wrong...)

We decided to not alter the metadata of a raster opened inside QGIS, because that raster could be opened multiple times as individual layers inside QGIS, e.g. image

Editing source metadata via layer A could mess up the usage of layer B.

In that way, the metadata of a raster can't be edited inside QGIS/EnMAP-Box. Note that the GDAL Metadata is also read-only inside the properties dialog. image

Currently you are suppose to only edit metadata outside of QGIS.

aloboa commented 2 years ago

ok to do it outside (although this is inconvenient for the user, who needs to visualize the band to decide), but in that case, unless there is an easily editable hdr file, which tool is available for the user to actually define the bad bands list?

janzandr commented 2 years ago

In case of an ENVI file, you can edit the .hdr file (note that you need to delete the .aux.xml file, if it already exists). Otherwise, you have to edit the *.aux.xml file. image

aloboa commented 2 years ago

Yes, that's the problem. I accept that you do not want to modify the metadata from within EnMapBox (although this is inconvenient for the user). I accept one solution, in this case, is editing the hdr, but this is not a good solution as most EnMapBox apps do not fully support the envi format (in the sense that not all the hdr information is taken into account). But editing the aux.xml is not a good solution because:

  1. The aux.xml can be silently modified by QGIS (perhaps also by EnMapBox?)
  2. Editing the aux.xml, with all these control characters, is very odd for the user. Precisely, the hdr survives because it is a simple text file, very easy to understand by the user.

Perhaps the bottom problem is that there is no good format file for hyperspectral images, other than the envi format, and gdal does not fully support the envi format (as not all hdr information is actually read).

Also, having a tool to create a bbl is needed, because manually writing the long vector of 0 and 1 elements is, in both cases, impractical and prone to errors. Currently, I use an R function in which I enter a vector with the positions of the bad bands and which returns the vector of 0 and 1 elements that I paste into the hdr. But this is really ugly. Why not do as I suggest within Raster Layer Styling/Gray and ask the user if the hdr should be modified and re-read?

janzandr commented 2 years ago

Yes, that's the problem. I accept that you do not want to modify the metadata from within EnMapBox (although this is inconvenient for the user). I accept one solution, in this case, is editing the hdr, but this is not a good solution as most EnMapBox apps do not fully support the envi format (in the sense that not all the hdr information is taken into account).

This is not true. For an ENVI raster, all the metadata from the .hdr is available to GDAL. You can edit the .hdr at any time outside of QGIS. BUT you have to delet the *.aux.xml file! If you do it that way, all the EnMAP-Box apps will see your metadata. If not, it is a bug.

But editing the aux.xml is not a good solution because: The aux.xml can be silently modified by QGIS (perhaps also by EnMapBox?) Editing the aux.xml, with all these control characters, is very odd for the user. Precisely, the hdr survives because it is a simple text file, very easy to understand by the user.

Yes, I understand that.

and gdal does not fully support the envi format (as not all hdr information is actually read).

That is not true. All information is available.

Also, having a tool to create a bbl is needed, because manually writing the long vector of 0 and 1 elements is, in both cases, impractical and prone to errors. Currently, I use an R function in which I enter a vector with the positions of the bad bands and which returns the vector of 0 and 1 elements that I paste into the hdr. But this is really ugly. Why not do as I suggest within Raster Layer Styling/Gray and ask the user if the hdr should be modified and re-read?

We have an algo, where you can edit metadata of a raster that is not opened inside QGIS. Not sure if that is convinient enought, image

janzandr commented 2 years ago

BTW - the algo from above is used by this app: image

aloboa commented 2 years ago

Envi format consists of a file with the raw bytes (let's call it the raw file) and an hdr file. Just these 2. If any program considers and even gives priority to a 3rd file (which, on top of it, has been silently created, the user is not aware of it), then you cannot claim that such a program is supporting the envi format.

As far as I understand, currently, the user will face the following confusing situation:

  1. The user has a hyperspectral image in envi format, raw + hdr files, in which no bad bands are listed.
  2. No aux.xml file exists
  3. The image is correctly read and displayed in EnMapBox
  4. Thanks to Raster Layer Styling/Gray, the user realizes that some bands are actually bad, and he/she writes down the corresponding band numbers.
  5. The user removes the image from EnMapBox.
  6. Using an external tool (which is kind of a shame), the user creates the long bbl of 0 and 1 and edits the hdr to include the bbl field.
  7. The user goes back to import and display the image but, as an aux.xml was silently (even treacherously) created in the previous display, EnMapBox ignores the bad bands Am I wrong?

Supporting the envi format means taking into consideration the raw and hdr files only . Perhaps you just do not want to support the envi format through all EnMapBox (I disagree but also understand the inconvenience of fighting aux.xml files from a dev point of view), except at import and display. But then this should be clear to the user: the user must import the image in envi format and then use Translate to create any other really supported format, which gdal metadata will be included in the aux.xml, and use this non-envi format to work with EnMapBox.

In my opinion, if an image in envi format is displayed in EnMapBox, the eventual aux.xml file should be ignored. If as a consequence of QGIS and/or GDAL oddities an aux.xml file is created for an image in envi format, that file should be automatically deleted when the image is removed or EnMapBox closed. aux.xml files are not compatible with the envi format because the user relies on the hdr.

jakimowb commented 2 years ago

I think it's quite obvious that the EnMAP-Box needs to be able to change bad band information in user-friendly way. This means a native-feeling GUI, in an interactive way and without manually editing hdr or xml files.

This is actually not the case, so we need to keep this issue open until there is a satisfying solution (no matter why we cannot provide a solution now or in the near future).

jakimowb commented 2 years ago

As @janzandr mentioned, there are a couple of problems related to the BBL support in QGIS/EnMAP-Box. In my humble opinion these are actually the following:

however, I am sure we will find a good solution for. Conceptually, the GDAL Metadata dialog can be enhanced to be made editable and write changed values back to local files if there is on file-lock activated. However, it's still questionable how such changes can be propagated back to already opened QgsRasterLayers.

janzandr commented 2 years ago

@jakimowb isn't that a good usecase for the GitHub "Convert to discussion" button? image

jakimowb commented 2 years ago

Why should it be?

janzandr commented 2 years ago

Because it needs discussion and a concept, and a decision if we want to alter raster sources inside QGIS at all.

The issue title "Interactive setting of bad bands in the Raster Layer Styling/Gray" isn't really reflecting that.

janzandr commented 2 years ago

I think it's quite obvious that the EnMAP-Box needs to be able to change bad band information in user-friendly way. This means a native-feeling GUI, in an interactive way and without manually editing hdr or xml files. This is actually not the case, so we need to keep this issue open until there is a satisfying solution (no matter why we cannot provide a solution now or in the near future).

If I don't remember wrong, I already had implemented a solution for editing raster metadata inside the EnMAP-Box GUI. It was implemented in a way, that if you edit a layer, all the layers that where also associated with the same source, where updated. In the background I disconnected the source from all the layers, did the edits inside the source, and reconnected the source to all the layers. For me it was fine, because a user could do that inside the GUI anyways: image

I removed that feature, because @jakimowb convinced me and the team, that this is dangerous behaviour.

Is that concern not valid anymore?

janzandr commented 2 years ago

I still think, that my approach is acceptable and doesn't contradict QGIS concepts. Every tool that is using a layer, would need to connect to the QgsMapLayer::dataSourceChanged signal, to aware of potential metadata changes in the background/by other tools.

janzandr commented 1 year ago

Hi @aloboa , me and @jakimowb decided to allow editing metadata of a raster source inside QGIS/EnMAP-Box. We now have a checkbox right next to the band selection widget: image

janzandr commented 1 year ago

Hi @aloboa, here is a test version: https://bitbucket.org/hu-geomatics/enmap-box/downloads/enmapboxplugin.3.11.20220912T114623.TEST.zip

aloboa commented 1 year ago

Hi @aloboa, here is a test version: https://bitbucket.org/hu-geomatics/enmap-box/downloads/enmapboxplugin.3.11.20220912T114623.TEST.zip

Yes, that would be it. But metadata and/or hdr must be updated. Perhaps a button with "update bbl" ?

aloboa commented 1 year ago

I think we (perhaps it's only me) are mixing the issue of adequately dealing with bad bands in EnMapBox no matter the actual format (which should remain here), with the issue of the proper support of the Envi format. Therefore, I open a new ticket for support of Envi format despite aux.xml files.

jakimowb commented 1 year ago

@aloboa I will make the GDAL Metadata Viewer editable (again).

I n case of raster sources that use the GDAL ENVI Driver, there will be special consideration of the ENVI hdr

It will be possible to modify band-specific values based on the QGIS expressions (similar to the Field Calculator that you can use to calculate values for vector layer attributes)

https://github.com/EnMAP-Box/qgispluginsupport/issues/7

However, due to potential side-effects (what happens to a QGIS layer if it's data source changes its meta data?), this needs a careful evaluation of the underlying mechanisms in QGIS / GDAL. To my feeling @janzandr is a bit too optimistic.