DOC: Integrate the content of the "/examples" directory into docs [Issue #396] #443

chmmao · 2023-05-09T21:04:23Z

Greetings,

This PR aims to address the issue #396.

I tried to integrate the examples shown in the examples directory into the docs for registry.
Not everything is integrated but I have chosen the most important ones in my opinion.
Hopefully it would resolve (partly) the issue, I am glad to hear improvements and potential mistakes.

bsipocz · 2023-05-10T07:14:32Z

Thank you so much, this is a great improvement. Could you please cull the examples/ content as part of this PR? Everything that you integrated over into the narrative docs, as well as anything that you find outdated or irrelevant.

chmmao · 2023-05-17T12:09:25Z

Thank you so much, this is a great improvement. Could you please cull the examples/ content as part of this PR? Everything that you integrated over into the narrative docs, as well as anything that you find outdated or irrelevant.

Yes, you can see in the changed files.
Basically I have integrated these files:
'images/ex_casA_image_cat.py'
'images/ex_get_cutouts.py'
'notebooks/simple_service_discovery.ipynb'
'notebooks/working_with_registry_results.ipynb'

In each file I have written comments about what part I exactly integrated into the registry docs (lines are given), what I think are unnecessary. I would not delete them right now, once everyone reviewing this PR agree, I will delete the files. In my opinion, all these 4 files can thus be deleted.

For the 'auth/' folder I am not sure what to do with them, as well as for the 'serialquery.py'.

andamian · 2023-05-17T16:34:34Z

@chmmao - thank you for picking this up!

I'm not sure what to do about the things in auth. While these examples should still work, auth is being revisited by the IVOA community and we might need to update it at some point. At the same time, a point was raised about service validators not being able to access authenticated services. If a solution is found, that might be applicable for the auth examples too but we are not there yet. I don't know what to do with them in the meantime. Move them to documentation without testing them? What do others think?

As for serialquery.py I think that can go unless @msdemlei disagree.

bsipocz · 2023-05-18T16:59:08Z

examples/images/ex_get_cutouts.py

@@ -24,3 +24,7 @@
    for image in results:
        print("Downloading %s..." % name)
        image.cachedataset(filename="NVSSimages/%s.fits" % name)
+
+# This file can be deletd


Please do delete the file (and the other similar ones, especially the notebooks) instead of commenting in it. The ultimate aim is to get rid of the examples directory altogether.

Ok sure, they are deleted now.

bsipocz · 2023-05-18T17:01:28Z

Move them to documentation without testing them? What do others think?

If they are expected to work, then yes, move them to the documentation with turning off testing. Keeping the examples directory, with absolutely untested code, and out of sight, is IMO not very useful. Better to keep the content in docs, and if testing is skipped on it, at least it's easy to grep for (and/or receiving user comments on it).

chmmao · 2023-05-24T13:45:04Z

Thanks for the review and for the suggestion you made:

Move them to documentation

I wonder which docs section should I move them to?

bsipocz · 2023-05-24T15:34:04Z

Practically you've already moved the content, I mean we should not keep notebook files anywhere in this repo.

bsipocz · 2023-05-31T18:55:18Z

docs/registry/index.rst

+.. doctest-remote-data::
+
+  >>> import pyvo as vo
+  >>> from astropy.coordinates import SkyCoord
+
+  >>> import warnings
+  >>> warnings.filterwarnings('ignore', module="astropy.io.votable.*")
+
+  >>> archives = vo.regsearch(servicetype='image', waveband='x-ray') 
+  >>> pos = SkyCoord.from_name('Cas A')
+  >>> len(archives)   # doctest: +IGNORE_OUTPUT
+  33


For example, this example generates a CI test failure, as due to the empty lines it is picked up as 3 separate examples and remote-data only added for the first one.

I would suggest to add >>> in the empty lines as well, so eventually everything stays as one example.

Suggested change

.. doctest-remote-data::

>>> import pyvo as vo

>>> from astropy.coordinates import SkyCoord

>>> import warnings

>>> warnings.filterwarnings('ignore', module="astropy.io.votable.*")

>>> archives = vo.regsearch(servicetype='image', waveband='x-ray')

>>> pos = SkyCoord.from_name('Cas A')

>>> len(archives) # doctest: +IGNORE_OUTPUT

33

.. doctest-remote-data::

>>> import pyvo as vo

>>> from astropy.coordinates import SkyCoord

>>>

>>> import warnings

>>> warnings.filterwarnings('ignore', module="astropy.io.votable.*")

>>>

>>> archives = vo.regsearch(servicetype='image', waveband='x-ray')

>>> pos = SkyCoord.from_name('Cas A')

>>> len(archives) # doctest: +IGNORE_OUTPUT

33

Thank you so much! I was wondering why it did not work since the code seem to be fine and docktest runs pretty well locally. Now the issue is fixed.

codecov · 2023-06-06T21:24:29Z

Codecov Report

Merging #443 (469c56a) into main (feda0f0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #443   +/-   ##
=======================================
  Coverage   79.98%   79.98%           
=======================================
  Files          52       52           
  Lines        6036     6036           
=======================================
  Hits         4828     4828           
  Misses       1208     1208

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

bsipocz

Please squash the example changing commits out from the commits, and fix the servicetype table, or let me know if you want me to do it.
Otherwise all looks good. Thank you!

bsipocz · 2023-07-06T21:10:03Z

docs/registry/index.rst

+
+  ===== ======================================
+  sia   Simple Image Access (SIA) services
+  ssa   Simple Spectral Access (SSA) services


Even if the aliases are dismissed from this list, do add sia2 to this table.
or maybe, instead link to the class docs ( as ~pyvo.registry.Servicetype) that has the list (e.g. eliminate the need to maintain such a list at two places).

bsipocz · 2023-07-06T21:10:49Z

docs/registry/index.rst

+
+.. doctest-remote-data::
+
+  >>> for service in archives:    # doctest: +ELLIPSIS


I suspect this should all work without the ELLIPSIS (as it should be opted in by default). If it's not, then it's an issue to be opened and followed-up.

bsipocz · 2023-07-06T21:11:49Z

docs/registry/index.rst

+
+.. doctest-remote-data::
+
+  >>> cats = vo.regsearch(keywords=['blazar','Fermi'])


Some style fixes are needed for the example, nothing serious, but adding and removing a few whitespaces here and there. I'm happy to do such fixes before merging.

I am very sorry for replying after such a long time, I have not been focusing on GitHub for a while. I have made changes such as:

use the ~pyvo.registry.Servicetype link for the parameters

some whitespaces fixes.
For this one:

.. doctest-remote-data::

for service in archives: # doctest: +ELLIPSIS

I used ELLIPSIS because the output is a very long list of services, I just took the first three of them as examples. I don't think without ELLIPSIS it would pass the test.

And... I tried to rebase/squash all commits into one, and somehow the result does not look like what I wanted. But the overall changes are still there, I would appreciate if you could help me here. :)

Re ELLIPSIS:

it should work as we enable it by default. If it doesn't then something in our doctest config went wrong, so I'll double check it.

And... I tried to rebase/squash all commits into one, and somehow the result does not look like what I wanted. But the overall changes are still there, I would appreciate if you could help me here. :)

Yeap, will do that (and will post the commands I use here, so you can check which one you did differently). Thanks for trying, and thanks for the rest of the fixes.

Re rebase, here is the big picture and the copy paste of what I see in my local setup:

locally you need the clone of astropy, and your fork, check it with git remote -v

fetch to update the remotes

rebase

push back to your remote branch.

$ git remote -v astropy git@github.com:astropy/pyvo (fetch) astropy git@github.com:astropy/pyvo (push) bsipocz git@github.com:bsipocz/pyvo (fetch) bsipocz git@github.com:bsipocz/pyvo (push) chmmao git@github.com:chmmao/pyvo (fetch) chmmao git@github.com:chmmao/pyvo (push) $ git fetch --all --prune Fetching bsipocz Fetching astropy remote: Enumerating objects: 1, done. remote: Counting objects: 100% (1/1), done. remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (1/1), 660 bytes | 660.00 KiB/s, done. From github.com:astropy/pyvo b96a950..feda0f0 main -> astropy/main Fetching chmmao $ git rebase -i astropy/main # editor comes up, do the commit reorders, squashes, removal of duplicates (if there is any from a previous rebase, etc). Note that we rebase on astropy/main and not on the main of your fork # # When rebase is finished (you may need to do a bunch of git rebase --continue, etc., push it back. $ git push chmmao HEAD:integrate_examples --force # note about the --force, it's needed as you overwrite the branch

Thank you so much! Good to know how to do that properly.

delete useless texts

… the narrative docs

bsipocz

Thank you @chmmao for this cleanup, I'm so glad to see the examples directory is gone and the content is now actually tested and exposed in the user documentation.

bsipocz added no-changelog-entry-needed documentation labels May 10, 2023

bsipocz reviewed May 18, 2023

View reviewed changes

bsipocz reviewed May 31, 2023

View reviewed changes

bsipocz reviewed Jul 6, 2023

View reviewed changes

bsipocz added this to the v1.5 milestone Jul 6, 2023

bsipocz force-pushed the integrate_examples branch from ebb99ea to 6cf2c3f Compare July 28, 2023 00:59

chmmao and others added 11 commits July 27, 2023 18:09

integrated examples into registry doc

119a2b9

improved use of phrase or words

d74d5c2

fix some words

8bbd8e9

fixed code examples;

064daaf

delete useless texts

optimised doctest flags

7e11110

Optimised doctest flags and code outputs

b0be6b7

fixing invalid code

8f02848

Removing example scripts and notebooks as their content is moved into…

bf59c8c

… the narrative docs

made code to fufill doctest

041c663

style fixes

b2d416a

DOC: cleanup unnecessary doctest directives

469c56a

bsipocz force-pushed the integrate_examples branch from 6cf2c3f to 469c56a Compare July 28, 2023 01:09

bsipocz approved these changes Jul 28, 2023

View reviewed changes

bsipocz merged commit 4bb146a into astropy:main Jul 28, 2023
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Integrate the content of the "/examples" directory into docs [Issue #396] #443

DOC: Integrate the content of the "/examples" directory into docs [Issue #396] #443

chmmao commented May 9, 2023

bsipocz commented May 10, 2023

chmmao commented May 17, 2023

andamian commented May 17, 2023

bsipocz May 18, 2023

chmmao May 24, 2023

bsipocz commented May 18, 2023

chmmao commented May 24, 2023

bsipocz commented May 24, 2023

bsipocz May 31, 2023

chmmao Jun 6, 2023

codecov bot commented Jun 6, 2023 •

edited

Loading

bsipocz left a comment

bsipocz Jul 6, 2023

bsipocz Jul 6, 2023

bsipocz Jul 6, 2023

chmmao Jul 26, 2023

bsipocz Jul 26, 2023

bsipocz Jul 28, 2023

chmmao Aug 15, 2023

bsipocz left a comment


		.. doctest-remote-data::

		>>> for service in archives: # doctest: +ELLIPSIS


		.. doctest-remote-data::

		>>> cats = vo.regsearch(keywords=['blazar','Fermi'])

DOC: Integrate the content of the "/examples" directory into docs [Issue #396] #443

DOC: Integrate the content of the "/examples" directory into docs [Issue #396] #443

Conversation

chmmao commented May 9, 2023

bsipocz commented May 10, 2023

chmmao commented May 17, 2023

andamian commented May 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz commented May 18, 2023

chmmao commented May 24, 2023

bsipocz commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 6, 2023 • edited Loading

Codecov Report

bsipocz left a 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

bsipocz left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 6, 2023 •

edited

Loading