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

Added Cheat Sheet #656

Merged
merged 17 commits into from
Jun 5, 2023
Merged

Added Cheat Sheet #656

merged 17 commits into from
Jun 5, 2023

Conversation

bewithankit
Copy link
Contributor

Resolves #591.

@bewithankit bewithankit added the documentation Improvements or additions to documentation label May 18, 2023
Copy link
Collaborator

@davidhassell davidhassell left a comment

Choose a reason for hiding this comment

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

Looks great. Some suggestions ...

docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
@sadielbartholomew
Copy link
Member

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.

@bewithankit
Copy link
Contributor Author

Thanks for the suggestions David!
Sadie, I'll add a few things and request a review from you!

@bewithankit bewithankit marked this pull request as ready for review May 19, 2023 10:48
@bewithankit
Copy link
Contributor Author

bewithankit commented May 19, 2023

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!

@bewithankit bewithankit added this to the 3.15.1 milestone May 19, 2023
Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

I've raised several minor comments, but overall this is looking excellent and will be very useful to users, I am sure. Let me know what you think, but this close to being merge-ready IMO...

docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
@sadielbartholomew
Copy link
Member

Thanks for addressing the feedback, Ankit. @davidhassell are you happy with the present state, in which case I think we can merge?

@bewithankit
Copy link
Contributor Author

bewithankit commented May 23, 2023

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.

@sadielbartholomew
Copy link
Member

Sounds good. I'm looking forward to seeing it in the documentation. Thanks Ankit.

Copy link
Collaborator

@davidhassell davidhassell left a comment

Choose a reason for hiding this comment

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

Hi Ankit - really liking this. A few comments ...

docs/source/cheat_sheet.rst Show resolved Hide resolved
| | >>> 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:* |
Copy link
Collaborator

Choose a reason for hiding this comment

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

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').

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

| | >>> 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:** |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| **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 ....

Copy link
Collaborator

Choose a reason for hiding this comment

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

Having just thought about the Cartesian case, what about

Regridding in spherical polar coordinates

Copy link
Contributor Author

@bewithankit bewithankit May 30, 2023

Choose a reason for hiding this comment

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

Hi David, I didn't get this! Can you explain?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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**

Copy link
Collaborator

@davidhassell davidhassell left a comment

Choose a reason for hiding this comment

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

Great. One minor suggestion - merge at will.

docs/source/cheat_sheet.rst Outdated Show resolved Hide resolved
@bewithankit
Copy link
Contributor Author

bewithankit commented Jun 5, 2023

Hi David, just wanted to confirm if I should merge it now or do it in the next release as we had discussed?

@davidhassell
Copy link
Collaborator

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)

@bewithankit bewithankit merged commit 6078e08 into NCAS-CMS:main Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation: printable cheat sheet including selected recipes
3 participants