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

[FB] [PI-3478] Metadata and cube maths documentation #3869

Merged

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Sep 18, 2020

🚀 Pull Request

Description

This PR adds user guide documentation support for the common metadata API from #3785.

This PR is in draft, with the following sections still to-do:

For reviewers, the rendered changes in this PR are available on readthedocs.

There is also a new Futher Topics chapter.


Consult Iris pull request check list

Copy link
Member

@rcomer rcomer left a comment

Choose a reason for hiding this comment

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

All this is nice and clear (and I love the smiley face attributes 😁), but I fear it's too much depth for the basic user. In fact, my hope is that lenient arithmetic (and other lenient operations hopefully added in future) will make metadata a thing that most users don't need to care about because it's handled for them under the hood.

The cylc tutorial has a Further Topics section that you can just go look at when you need those specific things. Maybe we could do similar with an "Advanced Topics" section - Metadata could go in there, and maybe the Real and Lazy Data section too.

docs/iris/src/userguide/metadata.rst Outdated Show resolved Hide resolved
@trexfeathers
Copy link
Contributor

Maybe we could do similar with an "Advanced Topics" section

@rcomer we have the Iris Technical Papers. If we decide things are too detailed for the User Guide (I haven't looked at this PR yet) then the Technical Papers would be an ideal home.

@rcomer
Copy link
Member

rcomer commented Sep 18, 2020

Thanks @trexfeathers that's a whole part of the docs I never knew existed!

@bjlittle
Copy link
Member Author

bjlittle commented Sep 19, 2020

@trexfeathers and @rcomer Yup, hear what you're both saying... and to be honest it feels like I've artificially squeezed this whole chapter in not quite the right place 👍

However, I don't think it really belongs in the tech papers section either. The tech papers for me, are more for iris devs, and the detail in this chapter is too high level for that audience - although a deeper treatment of suitable implementation detail would be appropriate for that.

I'll have a play, but I like the suggestion from @rcomer to have a Further Topics section... Which is almost like a middle ground between user guide and tech papers

@bjlittle
Copy link
Member Author

@rcomer We now have a Further Topics section, which I think works quite well... what do you think 🤔

@rcomer
Copy link
Member

rcomer commented Sep 20, 2020

“more advanced or curious user” - I like that 👍

@bjlittle bjlittle marked this pull request as ready for review September 21, 2020 07:53
@bjlittle bjlittle changed the title [FB] [PI-3478] Cube arithmetic docs userguide [FB] [PI-3478] Cube arithmetic documentation Sep 21, 2020
@bjlittle bjlittle changed the title [FB] [PI-3478] Cube arithmetic documentation [FB] [PI-3478] Metadata and cube maths documentation Sep 21, 2020
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.

Good improvements for consistency sake 👍

As for the docs I do want to preface my comments with the acknowledgement of just how subjective "good documentation" can be. Reviewing docs is hard ( and I did find reviewing this particularly hard) since you continually butt up against the problem of "that's not how I would do it". I think we tend to write the docs we would want to read, but we all have such different opinions of how we would want the docs to be. Personally, I like things to be as concise as possible, so I don't have to invest too much time in reading docs, before I can get back to writing code and feeling productive. So it's worth bearing that in mind, since I'm going to have a tendency to want to cut things out unless they are absolutely necessary.
I mention this so that you can take my comments with a large pinch of salt, as they are very much just my personal opinions and, no doubt, many would disagree with me. So I'm happy if you choose to ignore most of my comments included here.

That all being said, one thing I would be quite keen on addressing is the introduction to the metadata page. I find that that page reads a bit like a story in the sense of you don't know what will happen. I would prefer to include a bit of a summary of what will be covered in the introduction section. That way user's know what to expect and are more inclined to invest the time in reading it, or at least skip to the part that is relevant to them. Currently, the only way it is possible to read it is to just buckle down and read the whole thing.

docs/iris/src/further_topics/index.rst Outdated Show resolved Hide resolved
docs/iris/src/techpapers/index.rst Outdated Show resolved Hide resolved
lib/iris/common/metadata.py Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
@bjlittle
Copy link
Member Author

bjlittle commented Sep 22, 2020

@lbdreyer Cool.

Docs are hard, and can be interpreted quite differently by different people. I think that's normal. So, pinch of salt accepted, and understood 😉

That said, happy to add a preface that highlights what's to come, totally reasonable, and super helpful to the reader.

Personally, the difference here for me is the difference between a user guide and a reference. This isn't a reference, and I didn't intend it to be read like a reference. Akin to the rest of the docs in the user guide it's a narrative, but it highlights the different areas in the API that you should know about and may be useful to you - it's not about the best way to do things, as that really depends on what you're trying to do.

Personally, I tend to skim through a user guide and then constantly hit the reference for detail, but you need somewhere to set the context and scene, and for me, that's the user guide... otherwise you don't know what to look for in the reference.

Anyways, thanks for sticking with it and wading through... all comments welcome, and I'll service them as best I can 👍

Copy link
Contributor

@abooton abooton left a comment

Choose a reason for hiding this comment

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

Bill, I've had a look through all but the Lenient metadata documentation and I think it is really good; I think it is some of the clearest technical documentation I've read for a long time. I hope your pleased with it.
I have some really minor comments, and I've been uber picky on something that is subjective to begin with - so take them or leave them as you wish. You can stop reading now if you want!

In order to review the Metadata section I went back and re-read "Navigating a Cube" and "Basic cube maths". I'll add a few comments on those too as I noticed you are editing the files anyway. Also, I'm afraid I've not really read the previous review comments so I apologise if I mention something again.

  1. MetaData page
  • Overall I thought it was extremely clear. Just a few typos, I had minor suggestions for slicking up the introduction. It seemed like you really hit your stride when you got to the "Common Metadata API"!

  • We recommend that readers jump to the "Richer metadata section" - but its rather buried in the side bar.

  • It took me a while to follow the introduction and first section until I realised, I just needed to read the paragraphs in a different order! (You've already got all the vital information in there, so ignore my comments as you see fit. I doubt anyone else will notice, but I read very slowly, line by line)
    Personally I would:

  • Introduction: Move the first paragraph down, so all the class information is together, and make it more explicit by re-referencing the "Data structures doc" that definitively introduces Cube, AuxCoord and Coord classes.
    (Also it is a bit weird that we tell the user in the advanced section that "i.e. data about data" yet we don't explain it this simply in the "Basic User Guide"!)
    Introduction
    As discussed in Iris data structures, Iris draws heavily from the NetCDF CF Metadata Conventions as a source for its data model, thus building on the widely recognised and understood terminology defined within those CF Conventions by the scientific community.
    In Iris data structures we also introduced several classes in Iris that care about your data, and also your metadata: the Cube, the AuxCoord, and the DimCoord. In addition to these, Iris models several other classes of CF Conventions metadata: the AncillaryVariable (see Ancillary Data and Flags), the CellMeasure (see Cell Measures), and also the AuxCoordFactory (see Parametric Vertical Coordinate).

  • Common Metadata: I'd introduce what a metadata is first, and I'd introduce whats in the table before one tries to read it. So, I’d move the table to the end of the section, and the example describing it immediately before.
    More like:
    Common Metadata
    Each of the Iris CF Conventions classes use metadata to define them and give them meaning.
    A small collective of individual metadata members can be used to define the Iris CF Conventions classes. All the members reference specific CF Conventions terms, except from var_name and circular which are Iris specific terms. The individual metadata members used to define the various Iris CF Conventions classes are shown in Table 1.
    As Table 1 highlights, a specific subset of the metadata members are used to define and represent each of the Iris CF Conventions classes. Thus the metadata can be used to easily identify, compare and differentiate between individual class instances.
    For example, the collective metadata used to define an AncillaryVariable are the standard_name, long_name, var_name, units, and attributes members. Note that, these are the actual data attribute names of the metadata members on the Iris class.

  • All my other (tiny/minor) comments are on the files

  1. Document ordering
  • I really like having the advanced/detailed "Metadata" section in the "Further Reading". I did find that (having read lots) I’d overlooked “Further Topics” as I think I associate it as “Miscellaneous” topics, maybe because it has its own Introduction so I skimmed past it as separate to the user guide.
  • Maybe combine the introduction information into the main User Guide section?
    I wondered if the following is worth thinking about (not here) in the future:
  • Having the User Guide in a couple of parts, so we have one introduction in which we recommend users read Part1 which provides an essential orientation to Iris. For the advanced users, further topics are explained in Part 2? E.g.
  • User Guide - Part 1, User Guide - Part 2, User Guide - Technical Papers (in light of Ruth's comment about not having found them before)
    Or * User Guide: An Introduction, User Guide: Advanced/Further Topics, User Guide
  1. "Basic cube maths"
  • Title of "Basic cube maths" => "Cube maths"
    • Then the title is aligned with the other titles "Cube interpolation", "Cube statistics". (And I don't think 'basic' does it justice anymore! I think it will make it easier to spot in th euser guide)
  • When I started this section I felt like I was second guessing what was going on. I'll add some comments about the opening paragraph, to make the example being discussed more explicit. (Particularly as I think this epitomises what Iris helps the user best)
  1. LH side bar
  • I was having rather a lot of issues with the scrolling of the LHS bar - I don't think it is quite right.
  • I can't often see the top of the LHS bar, and it scrolls with the main documentation page. (I can show if you want)
  • It seems to work fine when looking at search results but not on documentation pages?
  1. Miscellaneous
  • I think I spotted inconsistent use of "meta-data" vs "metadata" in the User Guide
  • I wondered if an extra link in the "Basic cube maths" section to the "Metadata" section (can't remember if there is one) would be good for advertising all your fab work

docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Outdated Show resolved Hide resolved
docs/iris/src/userguide/cube_maths.rst Show resolved Hide resolved
Copy link
Member

@rcomer rcomer left a comment

Choose a reason for hiding this comment

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

Hi @bjlittle, I've just binge-read the lot! In general it all looks really clear to me. My comments are mostly just thoughts that occurred while reading and don't necessarily need any particular action.

docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/metadata.rst Show resolved Hide resolved
docs/iris/src/further_topics/lenient_maths.rst Outdated Show resolved Hide resolved
@rcomer
Copy link
Member

rcomer commented Sep 29, 2020

Looks good to me 😎

Update titles
Copy link
Contributor

@abooton abooton left a comment

Choose a reason for hiding this comment

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

This is looking really good.
I read the metadata section really thoroughly, and it reads really well.
I've given the Lenient metadata and the lenient maths a quick read. I think all the relevant information needed is here. So I'm going to merge it in 👍
As noted in my previous comment there were a couple of misc. items that we may wish to raise as follow on issues, so we can give the latter sections a thorough read at the same time.

:widths: auto
:align: center

============================================================================================= ================== ============
Copy link
Contributor

@abooton abooton Sep 29, 2020

Choose a reason for hiding this comment

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

I read this table a bit too quickly to begin I think...
At a glance, it looked like only the "standard_name", "long_name", "var_name" and the "attributes" that have the lenient property. However, re-reading it, I see that eg "measure" is only strict on CellMeasureMetadata.

I wonder whether it would be clearer if we had the equivalent of table 1, but indicating if strict/lenient ?

@abooton abooton merged commit d5a6dd4 into SciTools:cube-arithmetic-docs Sep 29, 2020
@bjlittle
Copy link
Member Author

Awesome, thanks!

bjlittle added a commit to bjlittle/iris that referenced this pull request Sep 30, 2020
trexfeathers pushed a commit that referenced this pull request Sep 30, 2020
* [FB] [PI-3478] Add resolve doc-strings (#3842)

* Add resolve doc-strings

* reorder some doctest statements

* [FB] [PI-3478] Metadata and cube maths documentation (#3869)

* fix title
@bjlittle bjlittle deleted the cube-arithmetic-docs-userguide branch September 30, 2020 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants