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

chore: add QuickStart section #171

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Conversation

uriii3
Copy link
Collaborator

@uriii3 uriii3 commented Oct 17, 2024

Reusing a notebook that we used for a presentation to have a QuickStart, with basic examples of usage and main tasks!

@uriii3 uriii3 changed the base branch from main to copernicusmarine-toolbox-v2 October 17, 2024 12:22
@uriii3 uriii3 requested a review from renaudjester October 17, 2024 14:47
Copy link
Collaborator

Choose a reason for hiding this comment

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

How that this work? this will be available also for the user when they are going to the website?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I tested the notebook is not dynamic since you cannot run anything I don't think those files (the .nc and .txt are useful at all)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, the .nc files and the .txt are needed to run the notebook. Then, I thought that if we wanted to update it, it would be better to have it there and not have to think 'Where did I save the files that I needed?'

It is for ease of use for ourselves, but of course if you prefer not having them there it is also an option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok nice :D

Let's keep it in some data folder or something then :D

Comment on lines 2 to 9
Quick start
=================

Under construction ...
.. toctree::
:maxdepth: 2
:caption: Contents:

.. image:: ../_static/under_construction.gif
:align: center
../_static/notebooks/CMT-english
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should treat the notebook directly as page of the website! so basically, you the notebook would be the quickstart page! here you are creating a subsection but there is no need

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mmh okey, that would make sense too!

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 there's quite a lot to talk about for the notebook:

  1. Please apply what we talked about: insitu plots not with curves but with points
  2. About the insitu plot, what is DOXY? We need to understand/explain the results of the commands
  3. I think the whole think can be reshaped to be more straight to the point and with better explained use cases (more like: what is I want the most recent data or to download a whole dataset). It's still missing some motivation I feel like.
  4. It needs to be rerun with the latest version (this can actually be a problem, maybe we could think of a solution to automate it)

Let's make it look a bit more like this: https://docs.xarray.dev/en/stable/getting-started-guide/quick-overview.html (maybe with more explanation and story but yeah)
Once all of this points are covered I will review it again

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. It was explicitly written down to be a line, that's why I was reluctant to change it. I asked and effectively it seems like it would be more correct to write it with points.
  2. DOXY stands for Dissolved OXYgen (I understand), but definitely not self-explainable. I lack a bit of knowledge on the INSITU part so it is a bit more difficult for me to be able to explain it, but we could take a look.
  3. Mmh I actually don't dislike that it is a little bit more pragmatic (and with less story telling). I mean, when doing a presentation I'm definitely on your side that greater story telling would be best and use it as the main focus and not as an excuse. In the QuickStart page, though, I feel like something like what it is now is a little in between from the xarray notebook you mention and the good 'story telling' notebook.
  4. Yep, didn't thought about it (I knew it had to be rerun but not thought that it would need to be rerun precisely before releasing) -> we would need to make something about it.

I like the xarray example!

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. ok let's change it
  2. Let's try to explain a bit more
  3. agreed
  4. yeah well it's not worse than keeping the documentation up to date even if we cannot make it automatic

"\n",
"For more information, see the [page about subset](subset-page) of the documentation. You can also check the dedicated pages for the [command line interface](cli-subset) or the {func}`python interface <copernicusmarine.subset>`.\n",
"\n",
"The subset is a powerful tool that allow you to benefit from the power the Copernicus Marine services. Indeed, not only do you have access to the whole catalogue but you also can pinpoint the data that interest you thanks to the two services: \"arco-geo-series\" and \"arco-time-series\". They are respectively optimised for retieving maps (short time spam, wide area) and time series (long time span, small area). "
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

typo: allows (no?)

"source": [
"## Copernicus Marine toolbox - Subset\n",
"\n",
"For more information, see the [page about subset](subset-page) of the documentation. You can also check the dedicated pages for the [command line interface](cli-subset) or the {func}`python interface <copernicusmarine.subset>`.\n",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the difference in the 'output' or how the two functions are presented might be a little weird, but for me it works. Just to keep it in mind.

README.md Outdated
@@ -86,7 +86,7 @@ Commands:

## Python package (API)

The `copernicusmarine` exposes a Python interface to allow you to [call commands as functions](https://toolbox-docs.marine.copernicus.eu/).
The `copernicusmarine` exposes a python interface to allow you to [call commands as functions](https://toolbox-docs.marine.copernicus.eu/).
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 python needs to always be with capital P 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's not the case in the rest of the documentation we need to change this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mmh python maybe yes, but I looked at how python interface was written in the code and it was the only instance with capital letter. I tried to keep the code consistent.

But if it is better expressed with capital letters I can change the whole code too (doesn't have that many instances).

@uriii3 uriii3 merged commit ed51cdf into copernicusmarine-toolbox-v2 Oct 22, 2024
2 checks passed
@uriii3 uriii3 deleted the add-notebook branch October 22, 2024 08:02
renaudjester pushed a commit that referenced this pull request Oct 28, 2024
Co-authored-by: renaudjester <[email protected]>
Adding an overview notebook for users to have something to start on to code with the toolbox.
renaudjester pushed a commit that referenced this pull request Oct 28, 2024
Co-authored-by: renaudjester <[email protected]>
Adding an overview notebook for users to have something to start on to code with the toolbox.
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.

2 participants