Ascat soilmoisture reader #1505

eysteinn · 2021-01-08T14:47:36Z

Reads Eumetsats soil moisture bufr message based on scatterometry data.
Tests are coming soon.

Tests added
Passes flake8 satpy
Fully documented
Add your name to AUTHORS.md if not there already

update from source

Add composite / enhancements for scatterometry soil moisture data

ghost · 2021-01-08T14:47:56Z

DeepCode's analysis on #dff630 found:

ℹ️ 1 minor issue. 👇

Top issues

Description	Example fixes
assertTrue cannot provide an informative message about the values in the comparison _ if it fails Occurrences: test_ascat_l2_soilmoisture_bufr.py:151 test_ascat_l2_soilmoisture_bufr.py:152 test_ascat_l2_soilmoisture_bufr.py:153	🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

codecov · 2021-01-18T14:44:36Z

Codecov Report

Merging #1505 (2e385c3) into master (e25415b) will increase coverage by 3.57%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master    #1505      +/-   ##
==========================================
+ Coverage   88.08%   91.65%   +3.57%     
==========================================
  Files         191      248      +57     
  Lines       29412    36171    +6759     
==========================================
+ Hits        25907    33152    +7245     
+ Misses       3505     3019     -486

Flag	Coverage Δ
behaviourtests	`4.36% <36.02%> (?)`
unittests	`92.19% <92.57%> (?)`

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

Impacted Files	Coverage Δ
satpy/enhancements/mimic.py	`0.00% <0.00%> (ø)`
satpy/modifiers/__init__.py	`100.00% <ø> (ø)`
satpy/modifiers/atmosphere.py	`50.00% <ø> (ø)`
satpy/modifiers/base.py	`80.00% <ø> (ø)`
satpy/modifiers/geometry.py	`83.87% <ø> (ø)`
satpy/modifiers/spectral.py	`92.39% <ø> (ø)`
satpy/multiscene.py	`90.76% <ø> (+1.44%)`	⬆️
satpy/node.py	`88.88% <ø> (-5.81%)`	⬇️
satpy/plugin_base.py	`88.00% <ø> (ø)`
satpy/readers/__init__.py	`97.40% <ø> (+2.84%)`	⬆️
... and 394 more

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 fc911e2...dff630a. Read the comment docs.

mraspaud

Thanks for adding this reader, I have some comments...

mraspaud · 2021-01-19T08:52:43Z

satpy/readers/ascat_l2_soilmoisture_bufr.py

+logger = logging.getLogger('ASCATSOILMOISTUREBUFR')
+
+
+class ASCATSOILMOISTUREBUFR(BaseFileHandler):


You should use CamelCase for class names

mraspaud · 2021-01-19T08:56:21Z

satpy/readers/ascat_l2_soilmoisture_bufr.py

+            while True:
+                # get handle for message
+                bufr = ec.codes_bufr_new_from_file(fh)
+                if bufr is None:
+                    break
+                ec.codes_set(bufr, 'unpack', 1)


I'm not familiar with bufr and eccodes, but will this read the file entirely? In which case, maybe the metadata and the data should be extracted in the same loop?

It will read the whole file but the file is usually small. The only metadata is the start / end date. The bufr data (channels) are optional and I don't think it fits with the metadata so I would read this separately even if it means scanning the file twice.

mraspaud · 2021-01-19T08:58:16Z

satpy/readers/ascat_l2_soilmoisture_bufr.py

+                years = np.resize(ec.codes_get_array(bufr, 'year'), size)
+                months = np.resize(ec.codes_get_array(bufr, 'month'), size)
+                days = np.resize(ec.codes_get_array(bufr, 'day'), size)
+                hours = np.resize(ec.codes_get_array(bufr, 'hour'), size)
+                minutes = np.resize(ec.codes_get_array(bufr, 'minute'), size)
+                seconds = np.resize(ec.codes_get_array(bufr, 'second'), size)
+                for year, month, day, hour, minute, second in zip(years, months, days, hours, minutes, seconds):
+                    time_stamp = datetime(year, month, day, hour, minute, second)
+                    date_min = time_stamp if not date_min else min(date_min, time_stamp)
+                    date_max = time_stamp if not date_max else max(date_max, time_stamp)


This could be factorized into a separate method for clarity.

I think I fixed this now, this has its own method.

mraspaud · 2021-01-19T09:00:32Z

satpy/tests/reader_tests/test_ascat_l2_soilmoisture_bufr.py

+    return retmsg
+
+
+msg = create_message()


This is a constant, so I would use upper case MSG

mraspaud · 2021-01-19T09:02:24Z

satpy/tests/reader_tests/test_ascat_l2_soilmoisture_bufr.py

+        self.assertTrue('start_time' in scn.attrs)
+        self.assertTrue('end_time' in scn.attrs)
+        self.assertTrue('sensor' in scn.attrs)
+        self.assertTrue('scatterometer' in scn.attrs['sensor'])


Would be nice to check the actual values of the attributes to make sure they are interpreted correctly

I added test for the values that are read from the file: start_time & end_time

mraspaud · 2021-01-19T09:03:12Z

satpy/tests/reader_tests/test_ascat_l2_soilmoisture_bufr.py

+        from satpy import Scene
+        fname = os.path.join(self.base_dir, FILENAME)
+        scn = Scene(reader='ascat_l2_soilmoisture_bufr', filenames=[fname])
+        scn.load(scn.available_dataset_names())


What about checking the result of the loading?

Now I check if all available dataset names are actually loaded.

mraspaud · 2021-01-19T09:03:32Z

satpy/tests/reader_tests/test_ascat_l2_soilmoisture_bufr.py

@@ -0,0 +1,180 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# Copyright (c) 2017-2019 Satpy developers


it's 2021 now :)

mraspaud · 2021-01-19T09:03:52Z

satpy/readers/ascat_l2_soilmoisture_bufr.py

@@ -0,0 +1,122 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# Copyright (c) 2019 Satpy developers


stickler-ci · 2021-01-21T12:16:28Z

satpy/tests/reader_tests/test_ascat_l2_soilmoisture_bufr.py

+    lat, lon = np.meshgrid(np.linspace(63, 65, nlat), np.linspace(-30, -20, nlon))
+    lat = np.round(np.ravel(lat), 4)
+    lon = np.round(np.ravel(lon), 4)
+    rstate=np.random.RandomState(0)


E225 missing whitespace around operator

eysteinn · 2021-01-25T14:19:33Z

I think all fixes are in

mraspaud

LGTM, thanks for adding the reader!

eysteinn and others added 3 commits December 18, 2020 23:20

Merge pull request #30 from pytroll/master

1797af9

update from source

Add reader for Eumetsats soil moisutre buffr messages

2d24b94

Add composite / enhancements for scatterometry soil moisture data

Remove unused lines.

8eb1db2

eysteinn requested review from djhoese and mraspaud as code owners January 8, 2021 14:47

Eysteinn added 7 commits January 8, 2021 14:56

Use safe file handling.

19ba19f

Changed to use ternary operator

48e4b6f

Remove reduntant datasets.

94f8833

Add test module for ascat soil moisture bufr product.

fcadf50

Remove test enhancements from generic.yaml

c5e2292

Fix problems pointed out by DeepCode.

ba6b519

Fix name of reader for testing purposes.

8c09a84

mraspaud requested changes Jan 19, 2021

View reviewed changes

Eysteinn added 4 commits January 20, 2021 09:40

Came case used for class names

7ea7f11

Minor code refactoring and additional tests.

10bbd86

Minor fixes required by deepcode.

cefbc1f

Use numpy RandomState.

2e385c3

stickler-ci reviewed Jan 21, 2021

View reviewed changes

Fix flake8 warnings.

dff630a

eysteinn requested a review from mraspaud January 25, 2021 14:18

mraspaud approved these changes Jan 25, 2021

View reviewed changes

mraspaud merged commit 0d4ef00 into pytroll:master Jan 25, 2021

eysteinn deleted the ascat_soilmoisture_reader branch January 25, 2021 14:52

mraspaud assigned eysteinn Jan 25, 2021

mraspaud added component:readers enhancement code enhancements, features, improvements labels Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ascat soilmoisture reader #1505

Ascat soilmoisture reader #1505

eysteinn commented Jan 8, 2021 •

edited

Loading

ghost commented Jan 8, 2021 •

edited by ghost

Loading

codecov bot commented Jan 18, 2021 •

edited

Loading

mraspaud left a comment

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

mraspaud Jan 19, 2021

eysteinn Jan 20, 2021

stickler-ci Jan 21, 2021

eysteinn commented Jan 25, 2021

mraspaud left a comment

		logger = logging.getLogger('ASCATSOILMOISTUREBUFR')


		class ASCATSOILMOISTUREBUFR(BaseFileHandler):

Ascat soilmoisture reader #1505

Ascat soilmoisture reader #1505

Conversation

eysteinn commented Jan 8, 2021 • edited Loading

ghost commented Jan 8, 2021 • edited by ghost Loading

DeepCode's analysis on #dff630 found:

Top issues

👉 View analysis in DeepCode’s Dashboard | Configure the bot

codecov bot commented Jan 18, 2021 • edited Loading

Codecov Report

mraspaud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eysteinn commented Jan 25, 2021

mraspaud left a comment

Choose a reason for hiding this comment

eysteinn commented Jan 8, 2021 •

edited

Loading

ghost commented Jan 8, 2021 •

edited by ghost

Loading

codecov bot commented Jan 18, 2021 •

edited

Loading