Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FY-4A AGRI support #77

Merged
merged 9 commits into from Aug 9, 2019

Conversation

@zxdawn
Copy link
Contributor

commented Jul 2, 2019

Add the FY-4A AGRI relative spectral responses.
The link of data: http://fy4.nsmc.org.cn/portal/cn/fycv/srf.html

@mraspaud

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

@zxdawn

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage remained the same at 73.211% when pulling b8acd27 on zxdawn:add_fy4a into 31d3dc2 on pytroll:master.

@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage remained the same at 73.211% when pulling 1f7c6cc on zxdawn:add_fy4a into 31d3dc2 on pytroll:master.

@adybbroe

This comment has been minimized.

Copy link
Collaborator

commented Aug 1, 2019

@zxdawn I finally took a look at your work. Sorry for the delay! It looks in general fine, only one small thing I found: The original RSR data are in nanometers for the three first bands, so there the default scale of 0.001 works fine (you get micrometers as expected by pyspectral). But for the rest of the bands the data appears to be in micrometers. So, there the scale should be 1.0.
I fixed it locally. Should I push? Not sure I am allowed to push to your fork though?

Also, I see that in your data, there is two files for the 3.75 band, though the RSR are identical. I suppose that is why the NWPSAF only has 13 bands and not 14 as in your case. I notice that band 7 and 8 on AGRI are different in terms of resolution, but looks like it is actually the same band in terms of sensor?

Fix band dependent scales
Signed-off-by: Adam Dybbroe <Adam.Dybbroe@smhi.se>
@adybbroe

This comment has been minimized.

Copy link
Collaborator

commented Aug 1, 2019

@zxdawn Maybe you can allow me to push to your fork?

@zxdawn

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

@zxdawn I finally took a look at your work. Sorry for the delay! It looks in general fine, only one small thing I found: The original RSR data are in nanometers for the three first bands, so there the default scale of 0.001 works fine (you get micrometers as expected by pyspectral). But for the rest of the bands the data appears to be in micrometers. So, there the scale should be 1.0.
I fixed it locally. Should I push? Not sure I am allowed to push to your fork though?

Also, I see that in your data, there is two files for the 3.75 band, though the RSR are identical. I suppose that is why the NWPSAF only has 13 bands and not 14 as in your case. I notice that band 7 and 8 on AGRI are different in terms of resolution, but looks like it is actually the same band in terms of sensor?

Yes, band 7 and band 8 are same for 3.72 um, except the resolution.
Enjoy pushing :)

adybbroe added 4 commits Aug 2, 2019
Updates requested by stickler-bot and update to version 1.0.7 of the …
…rsr data

Signed-off-by: Adam Dybbroe <Adam.Dybbroe@smhi.se>
Try fix for appveyor
Signed-off-by: Adam Dybbroe <Adam.Dybbroe@smhi.se>
revert latest change to travis file
Signed-off-by: Adam Dybbroe <Adam.Dybbroe@smhi.se>
Try fix appveyor
Signed-off-by: Adam Dybbroe <Adam.Dybbroe@smhi.se>

@adybbroe adybbroe added the enhancement label Aug 9, 2019

Revert back attempted fix to appveyor builds
Signed-off-by: Adam Dybbroe <Adam.Dybbroe@smhi.se>

@adybbroe adybbroe marked this pull request as ready for review Aug 9, 2019

@adybbroe

This comment has been minimized.

Copy link
Collaborator

commented Aug 9, 2019

I think we can merge, disregarding the failed appveyor builds for the time being - these should be fixed in another PR - it doesn't seem to have with this PR to do.

@adybbroe adybbroe merged commit c0ec583 into pytroll:master Aug 9, 2019

3 of 6 checks passed

Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
Scrutinizer Installing Code
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 73.211%
Details
stickler-ci No lint errors found
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.