Skip to content

Conversation

@LukeC92
Copy link
Contributor

@LukeC92 LukeC92 commented Aug 8, 2017

I have amended the documentation for the iris animate tool to make it explicitly clear which functions it will accept.

* plot_func (:mod:`~iris.plot` or :mod:`~iris.quickplot` plot):
Plotting function used to animate.
Plotting function used to animate. Must accept the signature
``plot_func(cube, vmin, vmax, coords)``. contourf, contour, pcolor and
Copy link
Member

Choose a reason for hiding this comment

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

It needs to be clear that the vmin, vmax and coords arguments are all supplied as keyword arguments (and thus can be in any order). It may also be wise to link to the functions in question as in

:func:`~iris.plot.contourf`

I can't remember if this works well for iris.quickplot as well as iris.plot, perhaps that can be included too?

Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

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

Looks like a useful enhancement.

Don't forget though, you will need to update the year date in the file header comments.
Iris actually has a test to check these : TestLicenceHeaders in iris/tests/test_coding_standards.py.

* plot_func (:mod:`~iris.plot` or :mod:`~iris.quickplot` plot):
Plotting function used to animate.
Plotting function used to animate. Must accept the signature
Copy link
Member

Choose a reason for hiding this comment

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

Not your fault, but I think the line above is nonsensical,
renders as "plot_func (plot or quickplot plot):".
I'd guess that should really be ""plot or quickplot function" ?

Copy link
Member

Choose a reason for hiding this comment

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

Remove the ~ from the front so that they render with full names, perhaps like this:

* plot_func (:mod:`iris.plot` or :mod:`iris.quickplot` plotting function)

``plot_func(cube, vmin=vmin, vmax=vmax, coords=coords)``.
:func:`~iris.plot.contourf`, :func:`~iris.plot.contour`,
:func:`~iris.plot.pcolor` and :func:`~iris.plot.pcolormesh`
all accept this signature by default.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about the last bit :
I don't think "by default" means anything in this case -- surely nothing can affect the valid function syntax ?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, perhaps we should say

... all conform to this signature

or

... all have this signature

@LukeC92 LukeC92 force-pushed the iris_animate_docs branch 3 times, most recently from f129a07 to d538fe9 Compare October 12, 2017 11:26
@LukeC92
Copy link
Contributor Author

LukeC92 commented Oct 12, 2017

I've rebased this branch off of the current Iris and it now seems to pass all of the tests. @ajdawson and @pp-mo can you see if all of your previous concerns have been addressed?

@pelson
Copy link
Member

pelson commented Oct 16, 2017

LGTM. Thanks @LukeC92.

@pelson pelson merged commit 1b69a30 into SciTools:master Oct 16, 2017
@QuLogic QuLogic added this to the v2.0 milestone Oct 16, 2017
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.

5 participants