Conversation
davidhassell
left a comment
davidhassell
left a comment
There was a problem hiding this comment.
Looks great. Some suggestions ...
Co-authored-by: David Hassell <davidhassell@users.noreply.github.com>
|
I'm aware this is still in draft form, but from a quick peek it is looking fantastic. If you'd like a second review (though probably not necessary) feel free to assign me when it is ready. |
|
Thanks for the suggestions David! |
|
Hi Sadie, I have marked it ready for review! You could either generate the documentation on your system or just head to https://github.com/bewithankit/cf-python/blob/cheatsheet/docs/source/cheat_sheet.rst where it renders properly! |
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
|
Thanks for addressing the feedback, Ankit. @davidhassell are you happy with the present state, in which case I think we can merge? |
|
Hi Sadie, thanks again for the suggestions. As the Cheat Sheet will be an addition to the sidebar menu, all the pages will need to be regenerated again. I think David said we'll add it in the next release as it'll be easier that way. |
|
Sounds good. I'm looking forward to seeing it in the documentation. Thanks Ankit. |
davidhassell
left a comment
davidhassell
left a comment
There was a problem hiding this comment.
Hi Ankit - really liking this. A few comments ...
| | | >>> jan_2023 = temp.subspace(T=cf.year(2023) & cf.month(1)) | | ||
| | | >>> annual_avg_61_90 = annual_global_avg.subspace(T=cf.year(cf.wi(1961, 1990))) | | ||
| +----------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | ||
| | **Accessing constructs** | *Select constructs by identity:* | |
There was a problem hiding this comment.
I think that the emphasis in this section should be on the named-by-construct methods (dimension_coordinate, auxiliary_coordinates, etc.) rather than the generic ones (construct, constructs). An awareness of the generic ones is good, but they're not really what people use, I think. E.g. I would hardly ever use f.constructs.filter_by_type('dimension_coordinate') instead of f.dimension_coordinates(); but I might well use f.constructs.filter_by_type('dimension_coordinate', 'field_ancillary').
There was a problem hiding this comment.
Thanks for the suggestion David. I thought users might not know all the metadata constructs so thought of putting the constructs container! I have done the changes, let me know if those are correct, thanks!
docs/source/cheat_sheet.rst
Outdated
| | | >>> d = f.constructs.filter_by_ncvar('time') | | ||
| | | >>> d = f.constructs.filter_by_ncvar('time', 'lat') | | ||
| +----------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | ||
| | **Regridding** | **Spherical regridding for regridding data between domains with spherical coordinate systems:** | |
There was a problem hiding this comment.
| | **Regridding** | **Spherical regridding for regridding data between domains with spherical coordinate systems:** | | |
| | **Regridding** | **Spherical regridding for regridding data between domains with spherical coordinate systems:** | |
This title (and the description in the tutorial) perhaps could be tweaked to make it clear that you can go to/from/between plane projections of spherical coordinate systems too. Can't think of anything pithy yet, though ....
There was a problem hiding this comment.
Having just thought about the Cartesian case, what about
Regridding in spherical polar coordinates
There was a problem hiding this comment.
Hi David, I didn't get this! Can you explain?
There was a problem hiding this comment.
Hi ANkit, sorry, it did get a bit confusing! I'm suggesting replacing **Spherical regridding for regridding data between domains with spherical coordinate systems:** with
**Regridding in spherical polar coordinates**
davidhassell
left a comment
davidhassell
left a comment
There was a problem hiding this comment.
Great. One minor suggestion - merge at will.
Co-authored-by: David Hassell <davidhassell@users.noreply.github.com>
|
Hi David, just wanted to confirm if I should merge it now or do it in the next release as we had discussed? |
|
Hi - I think that you can merge it now, it just won't show up till the next release when we rebuild the docs (well the it will appear in the side bar of index.html, but won't work - that's OK for a few days) |
3 participants
Resolves #591.