Full test coverage for GRIB1

[trexfeathers]

Closes #488

We need coverage to be file-based - we can't just mock the GribWrapper since we're planning to change to GribMessage. Probably can't mock eccodes either given how it manages a file cursor.

The files we have offer almost full coverage of our GRIB1 code (GribWrapper and _grib1_load_rules.py), but I will need to generate some artificial files (maybe by on-the-fly modification?) to fill the remaining gaps:

• iris-grib/src/iris_grib/__init__.py

  Line 446 in 11169cf

  pole_lat = -90

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 46 in 11169cf

  aux_coords_and_dims.append(

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 143 in 11169cf

  standard_name = "y_wind"

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 147 in 11169cf

  standard_name = grib._cf_data.standard_name

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 213 in 11169cf

  add_bounded_time_coords(aux_coords_and_dims, grib)

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 224 in 11169cf

  add_bounded_time_coords(aux_coords_and_dims, grib)

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 232 in 11169cf

  add_bounded_time_coords(aux_coords_and_dims, grib)

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 252 in 11169cf

  add_bounded_time_coords(aux_coords_and_dims, grib)

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 256 in 11169cf

  add_bounded_time_coords(aux_coords_and_dims, grib)

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 260 in 11169cf

  add_bounded_time_coords(aux_coords_and_dims, grib)

• iris-grib/src/iris_grib/_grib1_load_rules.py

  Line 299 in 11169cf

  aux_coords_and_dims.append(

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.80%. Comparing base (11169cf) to head (37f550b).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #583      +/-   ##
==========================================
+ Coverage   89.60%   89.80%   +0.20%     
==========================================
  Files           8        8              
  Lines        2473     2473              
  Branches      420      420              
==========================================
+ Hits         2216     2221       +5     
+ Misses        159      155       -4     
+ Partials       98       97       -1     

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

[trexfeathers]

I have discovered some of the timeRangeIndicator values in our code are actually not loadable. We rely on a special Eccodes key called startStep - this is generated by Eccodes and not part of the GRIB spec.

iris-grib/src/iris_grib/__init__.py

Lines 302 to 307 in 11169cf

	def _compute_extra_keys(self):
	"""Compute our extra keys."""
	global unknown_string
	self.extra_keys = {}
	forecastTime = self.startStep

How to calculate startStep and its other associated keys appears to rely on eccodes/definitions/grib1/localConcepts/edzw/stepType.def. Loading a file with a timeRangeIndicator value that is not included in stepType.def causes Eccodes to error if startStep is requested. The most basic demonstration can be seen when using the grib_get command:

$ grib_get my_file.grib1 -p startStep
ECCODES ERROR   :  Unknown stepType=[114] timeRangeIndicator=[114]
ECCODES ERROR   :  startStep (Function not yet implemented)

grib_get works fine when run on a file with a timeRangeIndicator value that IS included in stepType.def.

Our codebase has handling for the following: 1, 2, 3, 4, 5, 10, 51, 113, 114, 115, 116, 117, 118, 123, 124, 125. Of these, Eccodes errors for the below codes, which are precisely those not present in stepType.def:

• 51
• 114
• 115
• 116
• 117
• 125

I will therefore not attempt test coverage for loading these types of files. I will remove them from the original list.

[trexfeathers]

Found some more unreachable code. GribWrapper._get_verification_date() will only be called in circumstances when the value below does not begin with "time", i.e. 0, 1, 10:

iris-grib/src/iris_grib/__init__.py

Lines 56 to 74 in 11169cf

	TIME_RANGE_INDICATORS = {
	0: "none",
	1: "none",
	3: "time mean",
	4: "time sum",
	5: "time _difference",
	10: "none",
	# TODO #567 Further exploration of following mappings
	51: "time mean",
	113: "time mean",
	114: "time sum",
	115: "time mean",
	116: "time sum",
	117: "time mean",
	118: "time _covariance",
	123: "time mean",
	124: "time sum",
	125: "time standard_deviation",
	}

So my new tests can never exercise the code for the other timeRangeIndicator values. I will remove those points from the checklist.

[pp-mo]

OK I'm pretty convinced that this addresses the right areas, and I've run my own coverage check to confirm it does what you say. Good enough I think !