Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grib save : minor update #229

Merged
merged 2 commits into from
Nov 30, 2012
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions lib/iris/fileformats/grib_save_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,17 +264,23 @@ def hybrid_surfaces(cube, grib):

def non_hybrid_surfaces(cube, grib):

# pressure?
# pressure
if cube.coords("air_pressure") or cube.coords("pressure"):
grib_v_code = 100
output_unit = iris.unit.Unit("Pa")
v_coord = (cube.coords("air_pressure") or cube.coords("pressure"))[0]

# altitude
elif cube.coords("altitude"):
grib_v_code = 102
output_unit = iris.unit.Unit("m")
v_coord = cube.coord("altitude")

# # height?
# elif cube.coords("height"):
# grib_v_code = 102 # XXX is this correct?
# output_unit = iris.unti.Unit("m")
# v_coord = cube.coord("height")
# height
elif cube.coords("height"):
grib_v_code = 103
output_unit = iris.unit.Unit("m")
v_coord = cube.coord("height")

else:
raise iris.exceptions.TranslationError("Vertical coordinate not found / handled")
Expand Down Expand Up @@ -440,10 +446,10 @@ def data(cube, grib):

# mdi
if isinstance(cube.data, numpy.ma.core.MaskedArray):
gribapi.grib_set_long(grib, "missingValue", cube.data.fill_value)
gribapi.grib_set_double(grib, "missingValue", float(cube.data.fill_value))
data = cube.data.filled()
else:
gribapi.grib_set_long(grib, "missingValue", -1e9)
gribapi.grib_set_double(grib, "missingValue", float(-1e9))
data = cube.data

# values
Expand Down
2 changes: 2 additions & 0 deletions lib/iris/tests/test_grib_save.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,5 @@ def test_cube_with_time_bounds(self):

if __name__ == "__main__":
tests.main()


90 changes: 90 additions & 0 deletions lib/iris/tests/test_grib_save_rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# (C) British Crown Copyright 2010 - 2012, Met Office
#
# This file is part of Iris.
#
# Iris is free software: you can redistribute it and/or modify it under
# the terms of the GNU Lesser General Public License as published by the
# Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# Iris is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with Iris. If not, see <http://www.gnu.org/licenses/>.
"""Unit tests for iris.fileformats.grib_save_rules"""

# import iris tests first so that some things can be initialised before importing anything else
import iris.tests as tests

import gribapi
import numpy
import mock

import iris.cube
import iris.coords
import iris.fileformats.grib_save_rules as grib_save_rules


class Test_non_hybrid_surfaces(tests.IrisTest):
# Test grib_save_rules.non_hybrid_surfaces()

@mock.patch.object(gribapi, "grib_set_long")
def test_altitude_point(self, mock_set_long):
grib = None
cube = iris.cube.Cube([1,2,3,4,5])
cube.add_aux_coord(iris.coords.AuxCoord([12345], "altitude", units="m"))

grib_save_rules.non_hybrid_surfaces(cube, grib)

mock_set_long.assert_any_call(grib, "typeOfFirstFixedSurface", 102)
mock_set_long.assert_any_call(grib, "scaleFactorOfFirstFixedSurface", 0)
mock_set_long.assert_any_call(grib, "scaledValueOfFirstFixedSurface", 12345)
mock_set_long.assert_any_call(grib, "typeOfSecondFixedSurface", -1)
mock_set_long.assert_any_call(grib, "scaleFactorOfSecondFixedSurface", 255)
mock_set_long.assert_any_call(grib, "scaledValueOfSecondFixedSurface", -1)

@mock.patch.object(gribapi, "grib_set_long")
def test_height_point(self, mock_set_long):
grib = None
cube = iris.cube.Cube([1,2,3,4,5])
cube.add_aux_coord(iris.coords.AuxCoord([12345], "height", units="m"))

grib_save_rules.non_hybrid_surfaces(cube, grib)

mock_set_long.assert_any_call(grib, "typeOfFirstFixedSurface", 103)
mock_set_long.assert_any_call(grib, "scaleFactorOfFirstFixedSurface", 0)
mock_set_long.assert_any_call(grib, "scaledValueOfFirstFixedSurface", 12345)
mock_set_long.assert_any_call(grib, "typeOfSecondFixedSurface", -1)
mock_set_long.assert_any_call(grib, "scaleFactorOfSecondFixedSurface", 255)
mock_set_long.assert_any_call(grib, "scaledValueOfSecondFixedSurface", -1)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely use of mocking. Would it be worth while adding test coverage for a AuxCoord with bounds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, yes, but I thought that was out of scope for a support issue, where we're supposed to be providing evolutionary steps not full feature developments. I don't need bounds so I didn't think I was supposed to address them here, as it would needlessly delay the support. Not certain about this though!

I've requested a support milestone, which might help clarify this in the future.


class Test_data(tests.IrisTest):
# Test grib_save_rules.data()

@mock.patch.object(gribapi, "grib_set_double_array")
@mock.patch.object(gribapi, "grib_set_double")
def test_masked_array(self, mock_set_double, grib_set_double_array):
grib = None
cube = iris.cube.Cube(numpy.ma.MaskedArray([1,2,3,4,5], fill_value=54321))

grib_save_rules.data(cube, grib)

mock_set_double.assert_any_call(grib, "missingValue", float(54321))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you also be checking mock_set_double_array ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's just mocked because it's also called by the function being tested.
If we don't mock this, the real gribapi function is called with a mock grib message.

It could be argued that we should mock the gribapi instead of the functions,
but I couldn't work out how to do that.

@mock.patch.object(gribapi, "grib_set_double_array")
@mock.patch.object(gribapi, "grib_set_double")
def test_numpy_array(self, mock_set_double, grib_set_double_array):
grib = None
cube = iris.cube.Cube(numpy.array([1,2,3,4,5]))

grib_save_rules.data(cube, grib)

mock_set_double.assert_any_call(grib, "missingValue", float(-1e9))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto ... shouldn't you also be checking mock_set_double_array ?


if __name__ == "__main__":
tests.main()