search

robintw / Py6S

A Python interface to the 6S Radiative Transfer Model
GNU Lesser General Public License v3.0
191 stars 105 forks source link

Renamed the S2A band names and added S2B RSRs #51

Closed MarcYin closed 4 years ago

MarcYin commented 5 years ago

Renamed the S2A band names and added S2B RSRs, also changed the index of S3B RSRs to make it consistent with the oder of: S2A, S3A_OLCI, S3A_SLSTR, S2B, S3B_OLCI.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.04%) to 79.888% when pulling 1713228642d779dd1e6b4c05d61074ca3330f38f on MarcYin:patch-6 into fbf41a79fe8a36e0f822d2588b0112fbeaa982bc on robintw:master.

robintw commented 4 years ago

Hi - I'm trying to sort out various PRs for Py6S now, and can see three PRs from you about Sentinel spectral responses (#39, #40 and this one). Does this one (the most recently submitted), supersede the others?

Also, a few questions/comments:

  1. Could you add a comment with the source of the spectral response function data that you used to generate these SRFs?

  2. Why have you renamed the constants from something like S2A_MSI_01 to S2A_SR_AV_B1? The problem with renaming the constants is that it will break people's code that is based on a previous version of Py6S - and that's something I try to avoid where possible.

MarcYin commented 4 years ago

Yes, this one suppresses the others.

The source of this spectral response function is from Sentinel 2 official website: https://earth.esa.int/web/sentinel/user-guides/sentinel-2-msi/document-library/-/asset_publisher/Wk0TKajiISaR/content/sentinel-2a-spectral-responses

The reason for changing the name is because I just used the same naming convention from the original Sentinel 2 spectral response function file. There is one band named as B8A, which is weird ... but it is used for all the Sentinel 2 files and the original Py6S named it as band 9, which make it a bit confusing sometimes... so I would rather stick with the official naming conventions.

robintw commented 4 years ago

Thanks @MarcYin. I've merged the PR, but modified the names of the bands so that they match the names used for all the other sensors in Py6S - that is <satellite>_<sensor>_<band>. In this case, that's S2A_MSI_B01 etc. Thanks for pointing out the issue with Band 8A - I've kept that in there.

I've also added the source - although I forgot to do that in the merge, so added it as a separate commit later.

Thanks for contributing, and sorry for the very slow merge - life somewhat took over!

MarcYin commented 4 years ago

Happy to contribute to this great work!