search

CNES / MAJA

Level-2A processor used for atmospheric correction and cloud-detection. The active repository is the one below, this one is kept to leave access to the older issues.
https://gitlab.orfeo-toolbox.org/maja/maja
Apache License 2.0
137 stars 25 forks source link

Xml encoding in xml_tools.py #89

Open alceballosa opened 3 years ago

alceballosa commented 3 years ago

Hi,

I was recently testing MAJA in a Docker image (in Windows, via WSL2) prepared as per the tutorial here (using the installer): https://github.com/CNES/MAJA/issues/80

My miniconda Python environment consists of the following libraries: https://pastebin.com/LkY2XLDX

Most steps went smoothly, but during the xml_tools.py "can be opened?" validation, the lxml library could not open the XML files in the .SAFE folders so MAJA couldn't detect any assets, so I kept getting the "No input L1 product available" exception.

I fixed this by adding encoding='utf-8' to all instances of the open function in xml_tools.py; however, I am not sure if this would introduce bugs on non-WSL+Docker environments (maybe Windows is introducing some unwanted line endings?)

Should I make a pull request nonetheless?

Kind regards.

petket-5 commented 3 years ago

Hi,

Thank you for the analysis as well as the proposed solution. As of today, Maja under WSL is not officially supported but we added this to the internal todo-list. Anyhow, we would be happy to test the utf-8 fix using our CI attached to the gitlab. Could you open a MR for this under the following link please: https://gitlab.orfeo-toolbox.org/maja/maja

@jbrossar @Julien-Osman : Given the recent talks we had about the topic of contributions, are third party MRs like this eligible for CI testing?

Kind regards, Peter

alceballosa commented 3 years ago

Hi Peter!

Sorry for being late with this, the situation in my country (Colombia) has been delicate the last few weeks and I wasn't able to do the MR. In any case, just went ahead and did that so let me know if you are able to test the utf-8 fix.

Kindly,

Alberto.