Sbaf nn #97
Sbaf nn #97
Conversation
| default=None, | ||
| help=("Save only the AVHRR (1,2, 3B, 4, 5) channels to level1c4pps file. " | ||
| "And apply SBAFs to the channels.")) | ||
| help=(f"Save only the AVHRR (1,2, 3B, 4, 5) channels to level1c4pps file. " |
There was a problem hiding this comment.
f-string should only be used if you want to include variables, so remove the f from this row.
| help=("Save only the AVHRR (1,2, 3B, 4, 5) channels to level1c4pps file. " | ||
| "And apply SBAFs to the channels.")) | ||
| help=(f"Save only the AVHRR (1,2, 3B, 4, 5) channels to level1c4pps file. " | ||
| f"And apply SBAFs to the channels. The SBAF options are {[version for version in SBAF]}")) |
There was a problem hiding this comment.
The choices you have are already displayed in the help message by default, so not needed to be listed twice.
There was a problem hiding this comment.
Are they? I think something was added, but it threw an error, so I moved it into this help section
There was a problem hiding this comment.
What I mean is that the help message below describes the choices you have. To me it just adds noise if you add them also in the description.
$ python bin/vgac2pps.py --help
usage: vgac2pps.py [-h] [-o [OUT_DIR]] [-ne [NC_ENGINE]] [-all_ch] [-n19 {v2,v3,v4,v5,v6}] [-pps_ch] [-avhrr_ch] [-on [ORBIT_NUMBER]] [--don_split_files_at_midnight] fileN [fileN ...]
Script to produce a PPS-level1c file for a VIIRS level-1 scene
positional arguments:
fileN List of VIIRS files to process
optional arguments:
-h, --help show this help message and exit
-o [OUT_DIR], --out_dir [OUT_DIR]
Output directory where to store the level1c file
-ne [NC_ENGINE], --nc_engine [NC_ENGINE]
Engine for saving netcdf files netcdf4 or h5netcdf (default).
-all_ch, --all_channels
Save all 21 channels to level1c4pps file.
-n19 {v2,v3,v4,v5,v6}, --as_noaa19 {v2,v3,v4,v5,v6}
Save only the AVHRR (1,2, 3B, 4, 5) channels to level1c4pps file. And apply SBAFs to the channels.
-pps_ch, --pps_channels
Save only the necessary (for PPS) channels to level1c4pps file.
-avhrr_ch, --avhrr_channels
Save only the AVHRR (1,2, 3B, 4, 5) channels to level1c4pps file.
-on [ORBIT_NUMBER], --orbit_number [ORBIT_NUMBER]
Orbit number (default is 00000).
--don_split_files_at_midnight
Don't split files at midnight, keep as one level1c file.There was a problem hiding this comment.
And if python bin/vgac2pps.py --help throw an error for you, you should fix it :). I think this is not covered by any of the current test.
There was a problem hiding this comment.
But where does that help message come from?
There was a problem hiding this comment.
from the argparse library, so it is not our code.
There was a problem hiding this comment.
OK. Long story short. I'll remove the SBAF version info from the help argument
| @@ -66,7 +67,8 @@ | |||
|
|
|||
| MBAND_PPS = ["M05", "M07", "M09", "M10", "M11", "M12", "M14", "M15", "M16"] | |||
|
|
|||
| MBAND_AVHRR = ["M05", "M07", "M12", "M15", "M16"] | |||
| # "M10", "M14" are needed for NN SABAF, but are deleted before saving | |||
There was a problem hiding this comment.
comments about what happens in other parts of the code should be avoided.
There was a problem hiding this comment.
I changed the comment to
"M10", "M14" are not AVHRR channels, but needed for NN SABAF
| @@ -28,6 +28,7 @@ | |||
| import os | |||
| import time | |||
| from satpy.scene import Scene | |||
| from sbafs_ann.convert_vgac import convert_to_vgac_with_nn | |||
There was a problem hiding this comment.
this module is missing
There was a problem hiding this comment.
I don't know what to do about this.
There was a problem hiding this comment.
Ok. Nina is on vacation today, but I talked to her the last week. She said she plans to add this package to PyPI, but it is not yet there. When it is there we need to add it as a dependency in the setup.py file in the same way as e.g. satpy is listed, but until this is fixed all tests will fail on github even if you can run it locally...
| @@ -502,7 +572,14 @@ def process_one_scene(scene_files, out_path, engine="h5netcdf", | |||
| sensor = "viirs" | |||
| if noaa19_sbaf_version is not None: | |||
| sensor = "avhrr" | |||
| convert_to_noaa19(scn_, noaa19_sbaf_version) | |||
| if "NN" not in noaa19_sbaf_version: | |||
There was a problem hiding this comment.
I think the code will be more easy to test if you add this logic to the convert_to_noaa19 function. So I suggest that from this function you call convert_to_noaa19(scn_, noaa19_sbaf_version) as before.
There was a problem hiding this comment.
OK. I think I understand and I moved the logic into convert_to_noaa19
| } | ||
|
|
||
|
|
||
| def convert_to_noaa19_NN(scene, sbaf_version): |
There was a problem hiding this comment.
no capital letters in function name
| }, | ||
| "NN_v1": { | ||
|
|
||
| "datadir": "/nobackup/smhid20/proj/safcm/data/SBAFS_NN/", |
There was a problem hiding this comment.
we must include the network coefficient file in this package.
There was a problem hiding this comment.
Yes, that makes sense.
| if sbaf_version == "NN_v1": | ||
| sbaf_nn_dir = SBAF[sbaf_version]['datadir'] | ||
| else: | ||
| logger.error(f"Unrecognized NN version, {sbaf_version}") |
There was a problem hiding this comment.
I think you need logger.exception here, that will also raise an exception, otherwise you will end up with an uncontrolled exception later
pytest level1c4ppsflake8