satpy icon indicating copy to clipboard operation
satpy copied to clipboard

Enable NWCSAF-GEO composites for ABI, AHI, FCI

Open gerritholl opened this issue 5 years ago • 8 comments

Currently the NWCSAF-GEO composites only for for SEVIRI. This PR enables them for the other geostationary imagers supported by NWCSAF-GEO as well: ABI, AHI, and FCI.

  • [x] Closes #1134
  • [ ] Tests added
  • [x] Tests passed
  • [ ] Passes flake8 satpy
  • [ ] Fully documented

gerritholl avatar Apr 07 '20 08:04 gerritholl

Congratulations :tada:. DeepCode analyzed your code in 0.001 seconds and we found no issues. Enjoy a moment of no bugs :sunny:.

👉 View analysis in DeepCode’s Dashboard

ghost avatar Apr 07 '20 08:04 ghost

Currently this PR copy-pastes the YAML code from the seviri.yaml file to each of abi.yaml, ahi.yabl, and fci.yaml. This is a hard to maintain solution that violates Don't Repeat Yourself (DRY). I don't know how I can avoid that, seeing that YAML apparently does not support includes and that visir.yaml already contains composites with the same names but expecting nwcsaf-pps data. Pending a solution to that I've put this PR in draft mode. It works, it passes tests, but it's not a good solution yet.

gerritholl avatar Apr 07 '20 08:04 gerritholl

Coverage Status

Coverage decreased (-0.003%) to 89.605% when pulling 68e99a4ad8ac1f97f99a0f2436294608cd528bac on gerritholl:enable-nwcsaf-other-geo into 446af3ad7c21fac86bc0c8a8ff2f7a27bb91700f on pytroll:master.

coveralls avatar Apr 07 '20 08:04 coveralls

Codecov Report

Merging #1136 into master will decrease coverage by 0.00%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1136      +/-   ##
==========================================
- Coverage   89.60%   89.60%   -0.01%     
==========================================
  Files         200      200              
  Lines       29484    29484              
==========================================
- Hits        26420    26419       -1     
- Misses       3064     3065       +1     
Impacted Files Coverage Δ
satpy/scene.py 90.01% <0.00%> (-0.18%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2ce07c0...68e99a4. Read the comment docs.

codecov[bot] avatar Apr 07 '20 08:04 codecov[bot]

@pytroll/core This PR currently duplicates YAML code between seviri.yaml, abi.yaml, ahi.yaml, and fci.yaml (I dislike that, therefore it's still a draft). How can I adapt it to avoid this duplication, considering that visir.yaml is already occupied by nwcsaf-pps-based composites of the same name (but with differently named prerequisites)?

gerritholl avatar Apr 08 '20 16:04 gerritholl

From what I see we have four choices:

  1. What you're doing now.
  2. Move them to visir.yaml
  3. Refactor the composite loading to load all/most YAMLs (similar to what's been discussed in the various plugin issues/PRs). We could then make a YAML for nwcsaf-specific composites.
  4. Create a separate package that includes these configs similar to fogpy.

djhoese avatar Apr 08 '20 19:04 djhoese

Starting to clone and test the repository pytroll/satpy

rymdulf avatar Nov 20 '24 10:11 rymdulf

The testing process was executed successfully. See the test results for this pull request here!

rymdulf avatar Nov 20 '24 10:11 rymdulf