Skip to content

Conversation

@djkirkham
Copy link
Contributor

@djkirkham djkirkham commented Jun 29, 2017

Setting DimCoord points/bounds to an array that doesn't meet the requirements raises an exception, but the points/bounds array is still set on the coordinate. This PR stops that happening by doing the requirements checks before modifying the coord with the new array.

I've modified the functions _new_points_requirements() and _new_bounds_requirements() so that they check the slightly more lenient requirements of the array which is passed in, rather than those of the actual array assigned, i.e, the points value passed can be a scalar, and the bounds value can be 1D if there is only a single point.

@djkirkham djkirkham requested review from DPeterK and pp-mo June 29, 2017 15:39
@pelson
Copy link
Member

pelson commented Oct 17, 2017

👍
Thanks @djkirkham.

@pelson pelson self-requested a review October 17, 2017 10:48
@pelson pelson added this to the v2.0 milestone Oct 17, 2017
@pp-mo
Copy link
Member

pp-mo commented Oct 17, 2017

Practically, this all looks good + solid to me.

I don't see any need to test the new behaviour -- it is just clearly a fix.
However.. have you actually added some functionality in allowing scalar values, and is that tested ?

@djkirkham
Copy link
Contributor Author

No, scalar values were always allowed. It's just that previously the new array was being passed through _sanitise_array(), which ensured it was at least 1D (for points) or 2D (for bounds), before being checked.

@pp-mo pp-mo merged commit 447bc3a into SciTools:master Oct 17, 2017
@pp-mo
Copy link
Member

pp-mo commented Oct 17, 2017

Ok, I believe it !
Thanks @djkirkham

@djkirkham djkirkham deleted the dimcoord_check_before branch October 26, 2017 13:00
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.

3 participants