Skip to content

Conversation

stephenworsley
Copy link
Contributor

Fixes bug in #3340.

# Test circular longitude does not cause a crash.
res = 5
lat = DimCoord(np.arange(-90, 91, res), 'latitude',
units='degrees_north')

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

lat = DimCoord(np.arange(-90, 91, res), 'latitude',
units='degrees_north')
lon = DimCoord(np.arange(0, 360, res), 'longitude',
units='degrees_east')

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

u_arr = np.ones((nlat, nlon))
v_arr = np.ones((nlat, nlon))
u_cube = Cube(u_arr, dim_coords_and_dims=[(lat, 0), (lon, 1)],
standard_name='eastward_wind')

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

u_cube = Cube(u_arr, dim_coords_and_dims=[(lat, 0), (lon, 1)],
standard_name='eastward_wind')
v_cube = Cube(v_arr, dim_coords_and_dims=[(lat, 0), (lon, 1)],
standard_name='northward_wind')

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

@lbdreyer lbdreyer added this to the v2.3.0 milestone Aug 23, 2019
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

Nice addition @stephenworsley 👍
I believe the test should have an assertion of success/non-error?

@stephenworsley
Copy link
Contributor Author

@trexfeathers thanks.
As far as asserting success goes, the success case is that no exceptions are thrown. From what I can tell, in this case asserting there are no exceptions would be redundant. I've scanned through a couple other unit tests and it seems like there are a couple other unit tests without assertions (in iris/tests/unit/cube/test_Cube.py the test test_tolerance_f4() seems to be an example) though I'm still not entirely confident that I know exactly how such cases should be handled.

@trexfeathers
Copy link
Contributor

@trexfeathers thanks.
As far as asserting success goes, the success case is that no exceptions are thrown. From what I can tell, in this case asserting there are no exceptions would be redundant. I've scanned through a couple other unit tests and it seems like there are a couple other unit tests without assertions (in iris/tests/unit/cube/test_Cube.py the test test_tolerance_f4() seems to be an example) though I'm still not entirely confident that I know exactly how such cases should be handled.

OK, sounds good to me 😊

@trexfeathers
Copy link
Contributor

@pp-mo / @lbdreyer could this be merged please?

Copy link
Member

@lbdreyer lbdreyer left a comment

Choose a reason for hiding this comment

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

Thanks @stephenworsley !
Just a small change needed then this should be good to go

standard_name='eastward_wind')
v_cube = Cube(v_arr, dim_coords_and_dims=[(lat, 0), (lon, 1)],
standard_name='northward_wind')
u_cube.coord('longitude').circular = True
Copy link
Member

Choose a reason for hiding this comment

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

Could you also set circular=True for v_cube?
I know we don't explicitly do a compatibility check for the u_cube and v_cube as it is still a TODO but once that is fixed, it would throw up an error about the longitude coordinates being different on u_cube and v_cube.

Also, you could do it when you initialise the lon, i.e.:

lon = DimCoord(np.arange(0, 360, res), 'longitude',
                       units='degrees_east', circular=True)

As that would apply to both u_cube and v_cube

@lbdreyer
Copy link
Member

Great! Thanks @stephenworsley !

@lbdreyer lbdreyer merged commit d2e950a into SciTools:master Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants