Metop level 2 EUMETCAST BUFR reader #1079

TalfanBarnie · 2020-02-21T13:49:28Z

This PR provides a reader for IASI level 2 SO2 BUFR data distributed over EUMETCAST. Flake8 is still flagging some lines > 80 characters, but these are for test data which doesn't compromise readability of the code itself.

… up with one extra row

…ary files

mraspaud

Thanks for adding this new reader to satpy! Bufr isn't an easy format :)

I have a few comments, but please run flake8 with the flake8-docstring plugin on the code.

mraspaud · 2020-02-21T14:04:41Z

satpy/readers/iasi_l2_so2_bufr.py

+    import eccodes as ec
+except ImportError:
+    raise ImportError(
+        """Missing eccodes-python and/or eccodes C-library installation. Use conda to install eccodes""")


Just in case it's not eccodes itself missing but on of the requirements, you might want to append the original error message here.

mraspaud · 2020-02-21T14:05:37Z

satpy/readers/iasi_l2_so2_bufr.py

+data_center_dict = {3: {'name': 'METOP-1'}, 4:  {'name': 'METOP-2'},
+                    5: {'name': 'METOP-3'}}


Since you only have a name here, is it necessary to make it a nested dict ?

mraspaud · 2020-02-21T14:06:08Z

satpy/readers/iasi_l2_so2_bufr.py

+
+    def __init__(self, filename, filename_info, filetype_info, **kwargs):
+        """Initialise the file handler for the IASI L2 SO2 BUFR data."""
+


No blank line after a docstring.

mraspaud · 2020-02-21T14:07:00Z

satpy/readers/iasi_l2_so2_bufr.py

+        self.properties = {}
+        self.properties['start_time'] = start_time
+        self.properties['end_time'] = end_time
+        self.properties['SpacecraftName'] = data_center_dict[sc_id]['name']


For consistency: we usually call this metadata or mda for short, not `properties.

mraspaud · 2020-02-21T14:08:41Z

satpy/readers/iasi_l2_so2_bufr.py

+
+        fh.close()
+
+        return(start_time, end_time)


You don't need parenthesis here.

mraspaud · 2020-02-21T14:09:17Z

satpy/readers/iasi_l2_so2_bufr.py

+        return(start_time, end_time)
+
+    def get_attribute(self, key):
+        ''' Get BUFR attributes '''


Please use double quotes ("), and add a dot at then end.

mraspaud · 2020-02-21T14:13:56Z

satpy/tests/reader_tests/test_iasi_l2_so2_bufr.py

+SCAN_WIDTH = 120
+
+def save_test_data(path):
+    """Save the test file to the indicated directory"""


. at the end of the line.

coveralls · 2020-02-21T14:25:59Z

Coverage increased (+0.03%) to 88.919% when pulling 95ee8b8 on TalfanBarnie:master into ca8595f on pytroll:master.

codecov · 2020-02-21T14:27:08Z

Codecov Report

❗ No coverage uploaded for pull request base (master@6edfc45). Click here to learn what that means.
The diff coverage is 90.69%.

@@            Coverage Diff            @@
##             master    #1079   +/-   ##
=========================================
  Coverage          ?   88.93%           
=========================================
  Files             ?      194           
  Lines             ?    29826           
  Branches          ?        0           
=========================================
  Hits              ?    26525           
  Misses            ?     3301           
  Partials          ?        0

Impacted Files	Coverage Δ
satpy/tests/reader_tests/test_iasi_l2_so2_bufr.py	`96.15% <100%> (ø)`
satpy/tests/reader_tests/__init__.py	`98.5% <100%> (ø)`
satpy/tests/reader_tests/test_nucaps.py	`100% <100%> (ø)`
satpy/readers/nucaps.py	`89.04% <60.52%> (ø)`
satpy/readers/iasi_l2_so2_bufr.py	`94.44% <90.9%> (ø)`

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 6edfc45...95ee8b8. Read the comment docs.

…ong line number

… a variable in the test

stickler-ci · 2020-02-22T14:10:54Z

satpy/readers/iasi_l2_so2_bufr.py

+except ImportError as e:
+    raise ImportError(
+        """Missing eccodes-python and/or eccodes C-library installation. Use conda to install eccodes.
+           Error: """ , e)


E203 whitespace before ','

stickler-ci · 2020-02-23T12:20:57Z

satpy/tests/reader_tests/__init__.py

@@ -42,7 +42,8 @@
                                      test_ami_l1b, test_viirs_compact, test_seviri_l2_bufr,
                                      test_geos_area, test_nwcsaf_msg, test_glm_l2,
                                      test_seviri_l1b_icare, test_mimic_TPW2_nc,
-                                      test_slstr_l2, test_aapp_l1b, test_eps_l1b)
+                                      test_slstr_l2, test_aapp_l1b, test_eps_l1b, 


W291 trailing whitespace

…made to import it unless test is run

mraspaud

This looks good, I just have one last request I think.

satpy/readers/iasi_l2_so2_bufr.py

mraspaud · 2020-02-24T11:59:46Z

LGTM then, @djhoese do you want to have a look or can I merge ?

mraspaud · 2020-02-25T13:48:00Z

Merged, thanks for the contribution !

TalfanBarnie added 9 commits February 13, 2020 18:43

Added IASI level2 SO2 bufr reader .yaml and .py files

8fb05a3

working on IASI L2 SO2 tests - fixed mock bug where xr dataarry winds…

ea9bd31

… up with one extra row

CHECKPOINT: before converting test from using mocks to writing tempor…

cf17605

…ary files

Corrected problems found by linters

6edfc45

Merge remote-tracking branch 'upstream/master'

ecfade2

Deleted test file

50bab7a

Added name to AUTHORS.md

a3938b5

Added reader to index.rst

3e8b83b

Added docstring to iasi_l2_so2_bufr.py

41b4fa2

TalfanBarnie requested review from djhoese and mraspaud as code owners February 21, 2020 13:49

mraspaud requested changes Feb 21, 2020

View reviewed changes

mraspaud assigned TalfanBarnie Feb 21, 2020

TalfanBarnie added 6 commits February 21, 2020 16:07

Fixed flake8 --docstring-convention google errors

ca56545

Fixed eccodes error, nested dict

47b8504

Fixed odd docstring problem where extra blank line was reported at wr…

967dcd4

…ong line number

Fixed CodeFactor issues - trailing spaces in .yaml and use of file as…

b04d4bd

… a variable in the test

Fixed last trailing space in .yaml

425dd6b

Fixed error arising from trying to add exception object to string

b0e08f0

stickler-ci reviewed Feb 22, 2020

View reviewed changes

TalfanBarnie added 2 commits February 22, 2020 14:12

Fixed linting error introduced by fixing last error

3fac57e

Added level 2 IASI SO2 bufr tests to the reader tests init

650488b

stickler-ci reviewed Feb 23, 2020

View reviewed changes

TalfanBarnie added 3 commits February 23, 2020 12:33

Fixed linting error introduced by fixing last error.

80d4415

Now skips IASI l2 SO2 BUFR tests if platform is windows

af8c1f3

Moved import of eccodes inside file writer function so no attempt is …

95ee8b8

…made to import it unless test is run

mraspaud requested changes Feb 24, 2020

View reviewed changes

satpy/readers/iasi_l2_so2_bufr.py Show resolved Hide resolved

mraspaud approved these changes Feb 24, 2020

View reviewed changes

mraspaud added component:readers enhancement code enhancements, features, improvements labels Feb 24, 2020

mraspaud merged commit 811a939 into pytroll:master Feb 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metop level 2 EUMETCAST BUFR reader #1079

Metop level 2 EUMETCAST BUFR reader #1079

TalfanBarnie commented Feb 21, 2020

mraspaud left a comment

mraspaud Feb 21, 2020

mraspaud Feb 21, 2020

mraspaud Feb 21, 2020

mraspaud Feb 21, 2020

mraspaud Feb 21, 2020

mraspaud Feb 21, 2020

mraspaud Feb 21, 2020

coveralls commented Feb 21, 2020 •

edited

codecov bot commented Feb 21, 2020 •

edited

stickler-ci Feb 22, 2020

stickler-ci Feb 23, 2020

mraspaud left a comment

mraspaud commented Feb 24, 2020

mraspaud commented Feb 25, 2020

		data_center_dict = {3: {'name': 'METOP-1'}, 4: {'name': 'METOP-2'},
		5: {'name': 'METOP-3'}}


		def __init__(self, filename, filename_info, filetype_info, **kwargs):
		"""Initialise the file handler for the IASI L2 SO2 BUFR data."""

Metop level 2 EUMETCAST BUFR reader #1079

Metop level 2 EUMETCAST BUFR reader #1079

Conversation

TalfanBarnie commented Feb 21, 2020

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

coveralls commented Feb 21, 2020 • edited

codecov bot commented Feb 21, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mraspaud left a comment

Choose a reason for hiding this comment

mraspaud commented Feb 24, 2020

mraspaud commented Feb 25, 2020

coveralls commented Feb 21, 2020 •

edited

codecov bot commented Feb 21, 2020 •

edited