Small modification to documentation around cube constructor

[jkettleb]

Hello,

this is a very small modification. I wanted to try put it into a pull request on its own as its my first attempt at doing this. As it's small it hopefully won't waste a huge amount of anyone's time.

I have a couple of reservations about what I've done...

1. there is a comment in the code that talks about a temporary interface to Cube - and I'm not sure if the behaviour I've documented will be changed in the near future (and so maybe wasn't documented before on purpose)
2. I'm not sure whether I should have referenced the numpy documentation - this may be considered as revealing too much of the implementation detail (I don't know).

Jamie

[ajdawson]

Typo: by -> be

[jkettleb]

sorry Andrew - that was rubbish on my part.

[jkettleb]

I've also just noticed the little status line that suggests the Travis CI build is broken - is this something I've done (is the line with the reference too long for instance - and its failing some style checking?)

[ajdawson]

Typo: convertable -> convertible

This is not strictly correct though because array subclasses such as masked arrays are allowed, they are not converted with numpy.asarray(). The following might be more correct:

"If the object contains phenomenon values it can be a numpy array or array subclass, or any object that is convertible to an array using :func:numpy.asarray."

although I'm not sure if that is more detail than is needed. Perhaps some one else has thoughts on this.

[ajdawson]

@jkettleb - your update caused another comment to disappear, I've added it back.

The test fail is due to an error, but your long line probably would have caused the coding standards test to fail, don't worry though, the previous comment suggests you should remove it anyway.

[jkettleb]

@ajdawson - Hello Andrew, not sure what I did to remove that other comment (if know what I did so I can avoid doing it in future - that would be great.

I've revised the text based on your latest comments. I hope that reflects things more accurately.

[ajdawson]

@jkettleb - Don't worry about it, it was just because you made a change to the same line the comment was on so github considered both comments to refer to out of date code. You were just too quick!

[rhattersley]

Very minor (i.e. not a deal breaker for me), but you don't need the :py prefix here, Sphinx defaults to the Python domain.

[rhattersley]

NB. It looks like the CF standard name table has gone offline again 😦 so all the Travis tests are failing. But once we're happy with the PR we can just test on our own dev machines and merge.

[jkettleb]

I've removed the :py to be consistent with most other doc strings in iris. (I could find one other use of :py in ./iris/fileformats/grib/grib_save_rules.py, and its also in the advice used in the developers guide - e.g. http://scitools.org.uk/iris/docs/latest/developers_guide/documenting/rest_guide.html#creating-links, I don't know whether you want to change those or not for consistency. As @rhattersley said, its not a big deal.

[ajdawson]

I don't think this reads correctly, you shouldn't start a new sentence with "Where". We should keep this short and simple, I really preferred this:

If the object contains phenomenon values it can be a numpy array
or array subclass, or any object that is convertible to an array using
:func:`numpy.asarray`.

because it doesn't get too complicated, just an array or anything that is like an array, the following would be fine too:

If the object contains phenomenon values it can be a numpy array
or array subclass, or an *array_like* as described in
:func:`numpy.asarray`.

Remember that the reference to numpy.asarray will be turned into a link to the numpy documentation for that function in the online iris documentation, so users should have no problem finding what they need.

[jkettleb]

@ajdawson - Andrew - any better? English isn't my strong point (so I guess you could ask why am I trying to write documentation...)

[ajdawson]

Bang a full stop on the end and we're good to go I think.

[ajdawson]

@jkettleb - I personally think writing good documentation is one of the hardest parts of the project, it is always tricky to know what to write. Don't be disheartened at all, these types of contribution are extremely valuable to the project and we're really glad you took the time to do this.

I made one last comment about a full stop but other than that I'm happy.

[ajdawson]

Thanks @jkettleb. Since there are a lot of commits here would you mind squashing them into a single commit before this is merged. If you aren't sure how to do that the last section of this page in the developer guide explains how to do it. In your case you'd need to do:

git rebase -i 200f11bf

Which will pop open an editor where you leave the first line as it is and just replace the "pick" at the start of each other line with "f", save the file and your 7 commits should be squashed into a single commit. Please feel free to ask if you need any further guidance.

[ajdawson]

I'm guessing @jkettleb is a Met Office employee, in which case I'm also guessing his life has already been signed away and we don't need a CLA? Just want to check before merging @rhattersley, @munslowa?

[ajdawson]

@jkettleb - I forgot to say before... but it is always a good idea to back up your branch before doing the interactive rebase (squashing) in case you make a mistake.

[rhattersley]

I'm guessing @jkettleb is a Met Office employee, in which case I'm also guessing his life has already been signed away and we don't need a CLA?

Yup ... you guessed correct. 😉 @jkettleb is cleared for landing.

[jkettleb]

@ajdawson, Andrew - thanks for the tips on rebasing. I think I have done it - does it look ok from your view?

Thanks for the review and advice around this pull request.
Jamie

[pelson]

👍

[ajdawson]

Good work @jkettleb. As I said before writing good documentation is very hard and your efforts in this area are highly valued.