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

Basic support for varying longitude ranges. #350

Merged
merged 1 commit into from
Feb 15, 2013

Conversation

rhattersley
Copy link
Member

This pull request adds better support to iris.analysis.interpolate.linear for circular longitude ranges with different starting points.(*)

For example, starting with a Cube defined with longitudes covering the range [-180, 180) it is now possible to re-sample correctly with longitudes in the range [0, 360).

(*) Created in response to a user bug report where iris.analysis.interpolate.regrid was failing to deal with two Cubes with different longitude ranges.

from iris.cube import Cube


class Test_linear__circular_wrapping(tests.IrisTest):
Copy link
Member

Choose a reason for hiding this comment

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

There must be other interpolation tests somewhere? Is it worth putting this with those? Or moving those to here (in a separate PR?).

Copy link
Member Author

Choose a reason for hiding this comment

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

There must be other interpolation tests somewhere?

Indeed there are. And yes, it would be good to move them here in a separate PR (I've deliberately not done that in this PR to keep it obvious what is going on).

Copy link
Member

Choose a reason for hiding this comment

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

Is there a follow-on issue to do this?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is now ... #353 😉

@esc24
Copy link
Member

esc24 commented Feb 13, 2013

@rhattersley - looks like the gribapi build has fallen over on travis-ci. @pelson showed me that you can restart the build on travis-ci (for your own PRs) using the drop-down menu. Could you try this?

#
# You should have received a copy of the GNU Lesser General Public License
# along with Iris. If not, see <http://www.gnu.org/licenses/>.
"""
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what impact having a sub-folder will have our visual testing. I know cartopy can handle it, but not 100% that the standard mpl image testing functionality can. We can cross that bridge when we get to it though.

@pelson
Copy link
Member

pelson commented Feb 14, 2013

Could you try this?

bump.

@rhattersley
Copy link
Member Author

Could you try this?

bump.

Have done, but I don't hold out much hope - the build of the GRIB API has failed on my other completely unrelated PR (#354).

@rhattersley
Copy link
Member Author

    Could you try this?

bump.

Have done, but I don't hold out much hope - the build of the GRIB API has failed on my other completely unrelated PR (#354).

@rhattersley rhattersley reopened this Feb 14, 2013
@rhattersley
Copy link
Member Author

Could you try this?

bump.

Have done, but I don't hold out much hope - the build of the GRIB API has failed on my other completely unrelated PR (#354).

This time the travis build timed-out while waiting for apt-get update to work its magic.

Given we don't even have full test coverage on travis (and hence must not rely on travis anyway), please can we not hold up this PR because of some travis strangeness?

# Check that we can interpolate onto [0, 360) from a Cube
# defined over [-180, 180).
cube = self._create_cube([-180, -90, 0, 90])
samples = [('longitude', [0, 90, 180, 270])]
Copy link
Member

Choose a reason for hiding this comment

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

Please add some intermediate points (e.g. 45 deg and 315 deg) to test the linear interpolation. e.g. for 315 deg is it using the -90 and 0 values to estimate the desired sample point?

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps it's over the top 😉, but I've added quite a few more points (including intermediate 45 deg points) to see what happens with wrapping at each end.

And since we seem to be a gnat's whisker from merge, I've rebased & squashed.

Thanks.

@rhattersley
Copy link
Member Author

Given we don't even have full test coverage on travis (and hence must not rely on travis anyway), please can we not hold up this PR because of some travis strangeness?

Rebased to include #357 ... now working fine. :-)

esc24 added a commit that referenced this pull request Feb 15, 2013
Basic support for varying longitude ranges.
@esc24 esc24 merged commit 067ba2e into SciTools:master Feb 15, 2013
@rhattersley rhattersley deleted the wrap-linear branch February 19, 2013 14:06
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