fix: correct typos and improve doc #156

renaudjester · 2024-10-07T10:51:45Z

Based on some comments after reading https://toolbox-docs.marine.copernicus.eu/en/v2.0.0a2/

uriii3 · 2024-10-07T11:03:27Z

copernicusmarine/command_line_interface/group_subset.py

    .. code-block:: bash

-        copernicusmarine subset
-        --dataset-id cmems_mod_glo_phy-thetao_anfc_0.083deg_PT6H-i
-        --variable thetao
-        --start-datetime 2022-01-01T00:00:00
-        --end-datetime 2022-12-31T23:59:59
-        --minimum-longitude -6.17
-        --maximum-longitude -5.08
-        --minimum-latitude 35.75
-        --maximum-latitude 36.30
-        --minimum-depth 0.0
-        --maximum-depth 5.0
+        copernicusmarine subset --dataset-id cmems_mod_ibi_phy_my_0.083deg-3D_P1D-m --variable thetao --variable so --start-datetime 2021-01-01 --end-datetime 2021-01-03 --minimum-longitude 0.0 --maximum-longitude 0.1 --minimum-latitude 28.0 --maximum-latitude 28.1 --minimum-depth 1 --maximum-depth 2


weren't they saying also that they didn't like that ..code-block:: bash was appearing? is this solved on how to call the sphinx util?

You are right I should check this!

uriii3 · 2024-10-09T07:45:24Z

doc/command-line-interface.rst

 ===================================================

 .. click:: copernicusmarine.command_line_interface.copernicus_marine:base_command_line_interface
    :prog: copernicusmarine
-    :nested: full


what does this change?

So by manually adding them instead of letting sphinx-click do the nested documentation, it allows to skip some titles that were like cli-describe (it was a not from David)

uriii3 · 2024-10-09T07:46:48Z

doc/installation.rst

-* via docker `dockerhub`_
+* via pip (see `PyPI repository <https://pypi.org/project/copernicusmarine/>`_)
+* via mamba | conda (see `conda-forge channel <https://anaconda.org/conda-forge/copernicusmarine>`_)
+* via docker (see `dockerhub repository <https://hub.docker.com/r/copernicusmarine/copernicusmarine>`_)

 Alternatively, you can use a binary.


not for this PR (so, note to myself and you), but should we point somewhere here too? at the end of the section or something like that?

uriii3 · 2024-10-09T07:49:51Z

doc/installation.rst

 .. note::

    Note that the use of ``xarray<2024.7.0`` with ``numpy>=2.0.0`` leads to inconsistent results. See this issue: `xarray issue <https://github.com/pydata/xarray/issues/9179>`_.



how much can we change this? for example: xarray solves it in a new release: we don't release until one month in... will the documentation stay the same until we rerelease? or there is a way to update the documentation without releasing? is it good, then, to have this sort of comments here? (for me it is but maybe not the best way that we don't have a way to update the documentation)

But if they solve it in a new release then this statement is still true, isn't it?
So in which configuration would this become false?

Mmh if they solve it and then 2024.7.1 works (would be easier for users maybe?) or is it not that big of a priority?

If they solve it then 2024.7.1 works then it's good :D We still have to warn the users that if they use version 2024.7.0 it might bring some bugs with numpy>2.0.0 right?

uriii3 · 2024-10-09T07:53:52Z

doc/usage/describe-usage.rst

    copernicusmarine describe --include-datasets > all_datasets_copernicusmarine.json
+


here there is no response because it will all be sent to a json, but then we are not showing any sort of response... understandable from a logic point of view but maybe from a user side some answer would be more helpful? regarding what they said about obtaining responses always.

But I already show an example before and after so I thought this was enough :D And about that:

what they said about obtaining responses always.

It concerns the docstrings, not all the commands of the documentation

uriii3 · 2024-10-09T07:55:22Z

doc/usage/describe-usage.rst

+
+**Example:**
+
+If you want the `cmems_obs-ins_glo_phy-temp-sal_my_cora_irr` dataset only, you can use the following command:


Maybe: "If you want, for example, the comes... this way it takes importance of the name of the dataset(?)

it takes importance of the name? what does it mean?

that it is more easy to understand that the dataset id is random. It was a suggestion, definitely not super sure about it

uriii3 · 2024-10-09T07:56:53Z

doc/usage/get-usage.rst

    copernicusmarine get --dataset-id cmems_mod_ibi_phy_my_0.083deg-3D_P1Y-m --filter "*01yav_200[0-2]*"



the answer of this command too?

I added a little line to describe, I thought this was enough because in the end the response logs are not super relevant

uriii3 · 2024-10-09T07:57:28Z

doc/usage/get-usage.rst


 .. code-block:: bash

    copernicusmarine get --dataset-id cmems_mod_ibi_phy_my_0.083deg-3D_P1Y-m --filter "*01yav_200[0-2]*"

 Option ``--regex`` allows specifying a regular expression for more advanced file selection.

-**Example:**
+**Example** To download only files that contains "2000", "2001", or "2002" using a regular expression:

 .. code-block:: bash


maybe the answer for this one too?

cf answer from comment before

uriii3 · 2024-10-09T08:00:32Z

doc/usage/login-usage.rst

@@ -12,11 +12,11 @@ The ``login`` command creates a configuration file called ``.copernicusmarine-cr
    password :
    INFO - Configuration files stored in /Users/foo/.copernicusmarine

-If the ``.copernicusmarine-credentials`` file already exists, the system will ask for confirmation before overwriting it (using the ``--overwrite`` or ``--overwrite-configuration-file`` options).
+If the ``.copernicusmarine-credentials`` file already exists, the system will ask for confirmation before overwriting it. You can also use option ``–-overwrite`` or ``--overwrite-configuration-file`` to skip confirmation.


this makes my life weirder: we have a flag which is --skip-if-user-logged-in because the default behaviour is to precisely overwrite the credentials file... I'm not understanding this option then (or is this a new one which will change in favour of the other? In that case, I would be against this and maybe some check to see if it exists -but not overwrite it!-)

I still don't understand the --skip-if-user-logged-in but concerning the --overwrite here is how I understand the process:

You are a user, you do: copernicusmarine login, you log in as "USER1" and a file is created: ~/.../.credentials.
Then for some reason (for example you were on a common server) someone comes and wants to log in as "USER2".
They do copernicusmarine login and this will be prompt:

copernicusmarine password: File /../.credentials already exists, overwrite it ? [y/N]:

All good :D

Now USER1 and USER2 want to automatise this because they want to launch their scripts but they don't know who is connected. Hence they will use the --overwrite argument to avoid needed to prompt anything and let the code run!

Okay, then maybe it is worth to give it two thoughts...

From what I understand, and following your example: if user1 goes again in the machine, no one has touched it, and wants to login but not do it necessarily if it already has it's credentials, the flag --skip-if-user-logged-in precisely would skip the login for user 1.

What seems weird, though, is that this same example would be similar if user2 had logged in in the middle... so user1 would be using the wrong credentials (maybe not a big deal?)

Maybe to be thought but outside of the scope of this PR!

Yes we will study this when doing the PR on this subject

uriii3 · 2024-10-09T08:06:05Z

doc/usage/shared-options.rst

+      "output": "cmems_mod_ibi_phy_my_0.083deg-3D_P1D-m_thetao_19.00W-5.00E_26.00N-56.00N_0.51-5698.06m_1993-01-01-2021-12-28.nc",
+      "size": 210828.20248091602,
+      "data_needed": 210887.9328244275,
+      "coodinates_extent": {
+        "longitude": {
+          "minimum": -19.0,
+          "maximum": 4.999999046325684
+        },
+        "latitude": {
+          "minimum": 26.0,
+          "maximum": 56.0
+        },
+        "time": {
+          "minimum": "1993-01-01T00:00:00Z",
+          "maximum": "2021-12-28T00:00:00Z"
+        },
+        "depth": {
+          "minimum": 0.5057600140571594,
+          "maximum": 5698.060546875
+        }


the default would be now to not show this, no? will you update it when you update change the option?

uriii3 · 2024-10-09T08:07:09Z

doc/usage/subset-usage.rst

 About ``--bounding-box-method`` option
 """"""""""""""""""""""""""""""""""""""""


I will need to delete this!

(it's in the document I shared you I think)

correct a lot of typos and improve the doc

renaudjester requested a review from uriii3 October 7, 2024 10:52

uriii3 reviewed Oct 7, 2024

View reviewed changes

renaudjester requested a review from uriii3 October 8, 2024 14:36

uriii3 reviewed Oct 9, 2024

View reviewed changes

uriii3 approved these changes Oct 9, 2024

View reviewed changes

renaudjester added 2 commits October 9, 2024 10:23

fix: correct typos and improve doc

63035bb

fix: adding for example

5d60c1c

renaudjester force-pushed the correct-documentation branch from ddd2820 to 5d60c1c Compare October 9, 2024 10:08

test: update tests

1f8cb17

renaudjester merged commit 2909179 into copernicusmarine-toolbox-v2 Oct 9, 2024
2 checks passed

renaudjester deleted the correct-documentation branch October 9, 2024 12:50

renaudjester added a commit that referenced this pull request Oct 28, 2024

fix: correct typos and improve doc (#156)

4bc40d8

correct a lot of typos and improve the doc

renaudjester added a commit that referenced this pull request Oct 28, 2024

fix: correct typos and improve doc (#156)

b1ac03e

correct a lot of typos and improve the doc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct typos and improve doc #156

fix: correct typos and improve doc #156

renaudjester commented Oct 7, 2024

uriii3 Oct 7, 2024

renaudjester Oct 7, 2024

renaudjester Oct 7, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

uriii3 Oct 9, 2024 •

edited

Loading

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024 •

edited

Loading

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

uriii3 Oct 9, 2024

renaudjester Oct 9, 2024

		.. note::

		Note that the use of ``xarray<2024.7.0`` with ``numpy>=2.0.0`` leads to inconsistent results. See this issue: `xarray issue <https://github.com/pydata/xarray/issues/9179>`_.

		copernicusmarine describe --include-datasets > all_datasets_copernicusmarine.json


		Example:

		If you want the `cmems_obs-ins_glo_phy-temp-sal_my_cora_irr` dataset only, you can use the following command:

		copernicusmarine get --dataset-id cmems_mod_ibi_phy_my_0.083deg-3D_P1Y-m --filter "01yav_200[0-2]"

		About ``--bounding-box-method`` option
		""""""""""""""""""""""""""""""""""""""""

fix: correct typos and improve doc #156

fix: correct typos and improve doc #156

Conversation

renaudjester commented Oct 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uriii3 Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renaudjester Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uriii3 Oct 9, 2024 •

edited

Loading

renaudjester Oct 9, 2024 •

edited

Loading