Add atms runner by adybbroe · Pull Request #22 · pytroll/pytroll-cspp-runner

adybbroe · 2023-02-08T20:01:40Z

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

codecov · 2023-02-08T20:08:12Z

Codecov Report

Merging #22 (37a2d29) into main (6f22cca) will decrease coverage by 1.69%.
The diff coverage is 72.86%.

❗ Current head 37a2d29 differs from pull request most recent head 6205e1a. Consider uploading reports for the commit 6205e1a to get more accurate results

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
- Coverage   78.59%   76.91%   -1.69%     
==========================================
  Files          12       19       +7     
  Lines         995     1373     +378     
==========================================
+ Hits          782     1056     +274     
- Misses        213      317     +104

Flag	Coverage Δ
unittests	`76.91% <72.86%> (-1.69%)`	⬇️

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

Files	Coverage Δ
cspp_runner/config.py	`100.00% <100.00%> (ø)`
cspp_runner/constants.py	`100.00% <100.00%> (ø)`
cspp_runner/runner.py	`85.60% <100.00%> (+0.62%)`	⬆️
cspp_runner/tests/conftest.py	`100.00% <100.00%> (ø)`
cspp_runner/tests/test_config.py	`100.00% <100.00%> (ø)`
cspp_runner/tests/test_run_atms.py	`100.00% <100.00%> (ø)`
cspp_runner/viirs_dr_runner.py	`96.66% <ø> (ø)`
cspp_runner/logger.py	`0.00% <0.00%> (ø)`
cspp_runner/atms_rdr2sdr_runner.py	`58.60% <58.60%> (ø)`

... and 2 files with indirect coverage changes

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

# Conflicts: # cspp_runner/runner.py # cspp_runner/viirs_dr_runner.py

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

gerritholl

Good work. I have only minor comments and some questions. Some functions can do with better docstrings.

Could you edit the PR description itself to better summarise the changes?

gerritholl · 2023-05-23T07:12:09Z

bin/atms_dr_runner.py

+# Author(s):
+
+#   Adam Dybbroe <Firstname.Lastname at smhi.se>


This is redundant, as the authors are already listed in AUTHORS.md.

gerritholl · 2023-05-23T07:13:36Z

bin/atms_dr_runner.py

+_DEFAULT_LOG_FORMAT = '[%(levelname)s: %(asctime)s : %(name)s] %(message)s'
+
+
+LOG = logging.getLogger(__name__)


Not a constant, to logger would be a better name.

gerritholl · 2023-05-23T07:15:54Z

bin/atms_dr_runner.py

+    try:
+        atms = AtmsSdrRunner(cmd_args.config_file)
+    except Exception as err:
+        LOG.error('ATMS RDR to SDR processing crashed: %s', str(err))


Suggested change

LOG.error('ATMS RDR to SDR processing crashed: %s', str(err))

LOG.exception('ATMS RDR to SDR processing crashed: %s', str(err))

full traceback information is useful here, otherwise one might just get ...crashed: KeyError.

gerritholl · 2023-05-23T07:17:32Z

cspp_runner/atms_rdr2sdr_runner.py

+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+


This module is only for importing, so those lines are redundant.

gerritholl · 2023-05-23T07:17:55Z

cspp_runner/atms_rdr2sdr_runner.py

+# Author(s):
+
+#   Adam Dybbroe <Firstname.Lastname@smhi.se>
+


Authors are already listed in AUTHORS.md and cumbersome to maintain per module, so those lines could be removed here.

gerritholl · 2023-05-23T08:09:58Z

cspp_runner/tests/test_run_atms.py

+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+


Suggested change

#!/usr/bin/env python

# -*- coding: utf-8 -*-

Redundant lines for a module.

gerritholl · 2023-05-23T08:10:13Z

cspp_runner/tests/test_run_atms.py

+# Author(s):
+
+#   Adam Dybbroe <Firstname.Lastname@smhi.se>
+


Suggested change

# Author(s):

# Adam Dybbroe <Firstname.Lastname@smhi.se>

Redundant lines; authors are in AUTHORS.md.

gerritholl · 2023-05-23T08:13:42Z

cspp_runner/tests/test_run_atms.py

+def test_move_files_to_destination_pathlib(fake_yamlconfig_file, fake_sdr_homedir, fake_atms_sdr_files_one_pass):
+    """Test move the ATMS SDR files to a destination dir."""
+    config = read_config(fake_yamlconfig_file)
+    patterns = config['sdr_file_patterns']
+
+    sdr_file_paths = glob(str(fake_atms_sdr_files_one_pass / '*h5'))
+    expected = [os.path.basename(f) for f in sdr_file_paths]
+    expected.sort()
+
+    filelist = move_files_to_destination(sdr_file_paths, patterns, fake_sdr_homedir)
+
+    assert len(filelist) == 3
+    assert os.path.basename(os.path.normpath(os.path.dirname(filelist[0]))) == "noaa20_20230209_1317_27088"
+    bnames = [os.path.basename(f) for f in filelist]
+    bnames.sort()
+
+    assert bnames == expected


This function looks very similar to test_move_files_to_destination_dir_is_str. The two test functions can/should be merged into a single one that is parameterised.

gerritholl · 2023-05-23T08:15:20Z

examples/log_config.yaml_template

+  posttroll:
+    level: ERROR
+    propagate: false
+    handlers: [console,]


Does the trailing , do anything in YAML?

gerritholl · 2023-05-23T08:15:49Z

setup.py

-      author_email='adam.dybroe@smhi.se',
+      author='The Pytroll Team',
+      author_email='pytroll@googlegroups.com',
      classifiers=["Development Status :: 3 - Alpha",


Is it still Alpha if we're using it in production?

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

…nner into add-atms-runner

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

# Conflicts: # cspp_runner/runner.py

Signed-off-by: Adam.Dybbroe <a000680@c22526.ad.smhi.se>

Adam.Dybbroe added 3 commits February 8, 2023 11:12

Merge branch 'main' into add-atms-runner

a0eab76

First attempts to add support for ATMS SDR processing

4a1cd19

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Add tests to cover the subprocess call

b423729

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

adybbroe requested a review from gerritholl February 8, 2023 20:01

adybbroe self-assigned this Feb 8, 2023

adybbroe requested a review from TAlonglong February 8, 2023 20:01

adybbroe added the enhancement label Feb 8, 2023

adybbroe marked this pull request as draft February 8, 2023 20:03

Adam.Dybbroe added 11 commits February 9, 2023 10:57

Add verbosity option

6587ccd

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Remove pdb from code

43a0f58

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Fix call options for cspp/atms script

9873a17

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

create and send output messages when ATMS processing is ready

d41746a

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Add missing stop method

fb12b2a

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Add more debug info

17c4a48

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Fix correct orbit number in output messages

deb3a03

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Bugfix, and refactor orbit number estimation

37a2d29

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Temporarily fix path without schema and host specification

95060fa

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Improve logging doing less info and more debug

dc26f06

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Remove unused (commented out) code

517932a

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

adybbroe marked this pull request as ready for review May 2, 2023 09:08

Adam.Dybbroe and others added 5 commits May 4, 2023 14:21

Merge branch 'add-atms-runner' into smhi-atms

5c5b61a

# Conflicts: # cspp_runner/runner.py # cspp_runner/viirs_dr_runner.py

Add template for atms cspp configuration

a29bd84

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Merge branch 'add-atms-runner' into smhi-atms

233f54c

Merge branch 'add-option-to-turn-off-lut-updating' into smhi-atms

99ae1bc

Merge branch 'main' into add-atms-runner

cda5fd5

gerritholl requested changes May 23, 2023

View reviewed changes

Adam.Dybbroe added 3 commits October 9, 2023 10:31

Fix directory name from one of the three SDR files

470e316

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Merge branch 'add-atms-runner' of github.com:adybbroe/pytroll-cspp-ru…

f87b378

…nner into add-atms-runner

Fix test

6205e1a

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Adam.Dybbroe and others added 3 commits October 9, 2023 10:39

Merge branch 'add-atms-runner' into smhi-atms

7a4ef14

# Conflicts: # cspp_runner/runner.py

Add some more debug printouts

e9e399d

Signed-off-by: Adam.Dybbroe <a000680@c22526.ad.smhi.se>

Merge branch 'smhi-atms' into add-atms-runner

2c8afef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add atms runner#22

Add atms runner#22
adybbroe wants to merge 25 commits intopytroll:mainfrom
adybbroe:add-atms-runner

adybbroe commented Feb 8, 2023 •

edited

Loading

Uh oh!

codecov bot commented Feb 8, 2023 •

edited

Loading

Uh oh!

gerritholl left a comment

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

gerritholl May 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		_DEFAULT_LOG_FORMAT = '[%(levelname)s: %(asctime)s : %(name)s] %(message)s'


		LOG = logging.getLogger(__name__)

	LOG.error('ATMS RDR to SDR processing crashed: %s', str(err))
	LOG.exception('ATMS RDR to SDR processing crashed: %s', str(err))

Conversation

adybbroe commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gerritholl 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adybbroe commented Feb 8, 2023 •

edited

Loading

codecov bot commented Feb 8, 2023 •

edited

Loading