Refactoring and adding more tests by ninahakansson · Pull Request #129 · foua-pps/level1c4pps

ninahakansson · 2026-05-25T09:05:10Z

Refactoring.

Make one common function for saving data.
Make one function per sensor for loading and remapping data. This makes it possible to test all main functions.
Remove code for very old satpy versions.
Remove test-code not needed with pytest
Replace print with logging
Use the variables GEOLOCATION_NAMES and ONE_IR_CHANNEL for used in mersi for other converters.
Added tests
Use function to get pps, all, avhrr_heritage or default bands. Instead of many lists in every converter file.
Rename some variables

Closes #xxxx <!-- remove if there is no corresponding issue
Tests added
Tests passed: Passes pytest level1c4pps
Passes flake8
Add your name to AUTHORS.md if not there already

…(one common) Started to implement geolocation list as in mersi for other sensorrs (avhrr and slstr). Use function to get pps, all or default bands. Instead of many lists in every file. Added tests for main function in slstr, viirs, avhrr. Fixed tests

BengtRydberg

Nice MR. Always good to clean up. Added some minor type of comments. Did not comment everywhere since the same type of comment applies to many modules.

BengtRydberg · 2026-05-26T09:01:44Z

-BANDNAMES = list(PPS_TAGNAMES.keys())
+
+refl_bands = get_refl_bands(PPS_TAGNAMES)
+bandnames = sorted(list(PPS_TAGNAMES.keys()))


same as above

codecov · 2026-05-27T13:22:47Z

Codecov Report

❌ Patch coverage is 99.08497% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.72%. Comparing base (de5692d) to head (1807a5c).
⚠️ Report is 30 commits behind head on main.

Files with missing lines	Patch %	Lines
level1c4pps/eumgacfdr2pps_lib.py	95.55%	1 Missing and 1 partial ⚠️
level1c4pps/seviri2pps_lib.py	96.00%	1 Missing and 1 partial ⚠️
level1c4pps/tests/test_vgac2pps.py	95.23%	1 Missing ⚠️
level1c4pps/vgac2pps_lib.py	97.82%	1 Missing ⚠️
level1c4pps/viirs2pps_lib.py	97.72%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #129       +/-   ##
===========================================
+ Coverage   72.61%   93.72%   +21.10%     
===========================================
  Files          26       27        +1     
  Lines        2085     2166       +81     
  Branches      197      187       -10     
===========================================
+ Hits         1514     2030      +516     
+ Misses        523       93      -430     
+ Partials       48       43        -5

Flag	Coverage Δ
unittests	`93.72% <99.08%> (+21.20%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

BengtRydberg

Looks good. Added some minor comments mainly to make it more user friendly.

BengtRydberg · 2026-05-28T13:46:15Z

+    parser.add_argument('-ch', '--channel_selection', nargs='?',
+                        required=False, default="default",
+                        choices=["default", "all", "pps"],
+                        help="Channels to use.")


I think you should describe each option.

help="Channels to use. Choose from:\n" " default - Uses the standard channels\n" " all - Uses all available channels\n" " pps - Uses the channels required for PPS only" )

BengtRydberg · 2026-05-28T13:47:25Z

    parser.add_argument('-on', '--orbit_number', type=int, nargs='?',
                        required=False, default=0,
                        help="Orbit number (default is 00000).")
+    parser.add_argument('-ch', '--channel_selection', type=str, nargs='?',


same as above.

BengtRydberg · 2026-05-28T13:58:50Z

-                        help="Save all 20 channels to level1c4pps file.")
-    parser.add_argument('-pps_ch', '--pps_channels', action='store_true',
-                        help="Save only the necessary (for PPS) channels to level1c4pps file.")
+    parser.add_argument('-ch', '--channel_selection', nargs='?',


I think you should remove nargs. It is only confusing here.

BengtRydberg · 2026-05-28T14:01:48Z

-    parser.add_argument('-pps_ch', '--pps_channels', action='store_true',
-                        help="Save only the necessary (for PPS) channels to level1c4pps file.")
+    parser.add_argument('-ch', '--channel_selection', nargs='?',
+                        required=False, default="default",


not needed, since this happen automatically if argument starts with -

BengtRydberg · 2026-05-28T14:02:08Z

-                        help="Save all 20 channels to level1c4pps file.")
-    parser.add_argument('-pps_ch', '--pps_channels', action='store_true',
-                        help="Save only the necessary (for PPS) channels to level1c4pps file.")
+    parser.add_argument('-ch', '--channel_selection', type=str, nargs='?',


same as above

BengtRydberg · 2026-05-28T14:14:01Z

+def get_band_names(pps_tags_dict, mode="default"):
+    """Get bands to use from all, default and recommended for pps."""
+    PPS_BANDS = ['ch_r06', 'ch_r09', 'ch_r13', 'ch_r16', 'ch_tb37', 'ch_tb85', 'ch_tb11', 'ch_tb12']
+    AVHRR_BANDS = ['ch_r06', 'ch_r09', 'ch_r16', 'ch_tb37', 'ch_tb11', 'ch_tb12']
+    if mode == "all":
+        return sorted(list(pps_tags_dict.keys()))
+    if mode == "pps":
+        return [key for key in pps_tags_dict if pps_tags_dict[key] in PPS_BANDS]
+    if mode == "avhrr_heritage":
+        return [key for key in pps_tags_dict if pps_tags_dict[key] in AVHRR_BANDS]
+    return [key for key in pps_tags_dict if "xx" not in pps_tags_dict[key]]


The logic is a bit hidden, more clear with something like:

PPS_BANDS = {'ch_r06', 'ch_r09', 'ch_r13', 'ch_r16', 'ch_tb37', 'ch_tb85', 'ch_tb11', 'ch_tb12'} AVHRR_BANDS = {'ch_r06', 'ch_r09', 'ch_r16', 'ch_tb37', 'ch_tb11', 'ch_tb12'} def get_band_names(pps_tags_dict, mode="default"): """Get bands to use based on mode selection.""" if mode == "all": return sorted(pps_tags_dict) if mode == "pps": return [key for key, val in pps_tags_dict.items() if val in PPS_BANDS] if mode == "avhrr_heritage": return [key for key, val in pps_tags_dict.items() if val in AVHRR_BANDS] if mode == "default": return [key for key, val in pps_tags_dict.items() if "xx" not in val] raise ValueError(f"Unknown mode: {mode}")

BengtRydberg · 2026-05-28T14:16:06Z

+
+def get_refl_bands(pps_tags_dict):
+    """Get reflective bands."""
+    return [key for key in pps_tags_dict if "ch_r" in pps_tags_dict[key]]


simpler with:
return [key for key, val in pps_tags_dict.items() if "ch_r" in val]

BengtRydberg

I think it looks good! I left some comments yesterday, that will improve the user experience, that you could have a look at if you have the energy.

ninahakansson added 21 commits May 21, 2026 13:36

Remove test code not needed with pytest

a97782d

flake8

911316c

Replace print with logging

3174f33

Remove superfluous documentation

7aea900

Remove code for unsupported satpy <0.26

92c08f9

Replaced scn_ with scene as in mersi2pps_lib

84b0dc2

Add GEOLOCATION_NAMES to modis and metimage

99463aa

Fix some mistakes in ISCCPNG reader and add tests

1eaeb44

Add tests for mersi, modis and metimage

a5d75ea

fix fci reader, it fails if data is loaded in separate function

d0b5ddb

Add ONE_IR_CHANNEL var to all converters

56c2f52

autopep8

264ea70

autopep8

2042386

autopep8

e10d189

flake8

d962a74

Fix vgac tests

e0806d5

fix mistake

1ce9242

Remove load data function from fci reader and add test

604f4db

Mock some tests differently

ae04b9e

autopep8

55f4e2f

ninahakansson requested a review from BengtRydberg May 25, 2026 12:20

ninahakansson added 6 commits May 25, 2026 14:41

Updated README

04bd97b

Remove old code and comments

6cde6ce

Update two tests

8aed261

Remove check for very old satpy

9d6680e

Remove check for old satpy version

99275de

Updated test

16acfa6

BengtRydberg reviewed May 26, 2026

View reviewed changes

Switch to official coveralls github action

8e95267

ninahakansson added 10 commits May 27, 2026 09:38

Rename vars

730a4ed

Flake8

93a5fbe

Fix tests

14292ea

Test main seviri function

8c59e13

Use the check_file_exists in all converters

bfb1c66

Format exception messages nicer

8ab833f

Test files are missing

4c77c2b

Modify some test

83e2370

autopep8

dfceb10

isort imports

aed7baf

ninahakansson added 9 commits May 28, 2026 09:37

Rename irch

007d167

Fix test

acc451a

Test the other AVHRR format

1e2b4b4

fix mersi test

63ebc68

Check that we got the correct MBANDS

dea84fd

autopep8

2b946f1

Fix tests

baef4e9

autopep8

1807a5c

Add avhrr_heritage to slstr and fci converter

6502b4b

BengtRydberg reviewed May 28, 2026

View reviewed changes

ninahakansson added 2 commits May 28, 2026 16:25

ruff

cd2da5e

Fix error

920f797

BengtRydberg approved these changes May 29, 2026

View reviewed changes

ninahakansson added 5 commits May 29, 2026 11:18

Fix eumgacfdr reader

394dcb2

fix_timestamp_datatype will get the encoding needed

912c509

Cahgne tests to avoid warnings

2e3f3ad

Merge branch 'main' into cleaning

dec3468

Fix review comments

6108b98

ninahakansson merged commit e9253bc into foua-pps:main Jun 1, 2026
8 checks passed

Uh oh!

Conversation

ninahakansson commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BengtRydberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BengtRydberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BengtRydberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ninahakansson commented May 25, 2026 •

edited

Loading

codecov Bot commented May 27, 2026 •

edited

Loading