[SYNPY-1345] Migrate to mkdocstrings #1023

BryanFauble · 2023-12-11T22:32:57Z

Problem:
Our previous documentation tool through Sphinx was a bit difficult to work with. It relied on RST files. It used sphinx docstrings format as well.

Solution:

Migrated the pages to mkdocstrings and python.
Migrated a small number of RST documents into markdown - More documents still need to be converted over, but those can come when we start to write content. The amount done as of now is to showcase what the documentation looks like in this format.
Moved to google docstrings for object-orientated POC. Tested out what the google docstring looks like in the Synapse class.

Testing:

Make sure that you have the latest dependencies installed. ie: pip install .
In the root synapsePythonClient directory you should see the mkdocs.yml. In this directory you can run mkdocs serve to view the live files.

mkdocs builds is used to create the site directory when we serve these on a live server.

BryanFauble · 2023-12-11T22:35:32Z

mkdocs.yml

+repo_name: synapsePythonClient
+
+# Navigation
+nav:


This nav section is the biggest thing to be concerned about for this ticket. This is everything for the first round of documentation that I thought we should include. This is based off the documentation from https://diataxis.fr/

What do you think about adding navigation.tabs and/or navigation.tabs.sticky feature? Because the Python client docs is pretty big, I think adding tabs could be useful / more organized.

For some references, both mkdocstrings and Material for MkDocs use it for their docs site.

I like navigation.tabs!

I'm thinking without sticky so that more of the page is viewable in longer articles.

Without sticky:

With sticky:

BryanFauble · 2023-12-11T22:36:14Z

synapseclient/client.py

-                                  when making http requests
-
-    Typically, no parameters are needed::
+    Attributes:


This is what it'll look like to convert the existing docstrings into google style. This style gives us the biggest flexibility and interoperability with mkdocstrings.

Agreed, I think we can have an epic around converting everything to google style docs, and those can be good-first-tickets for the community to contribute to. They're also honestly just a lot easier to write...

It'll also be good tickets for all of us to contribute.

BryanFauble · 2023-12-11T22:37:12Z

synapseclient/models/file.py

@@ -21,14 +21,49 @@

 @dataclass()
 class File:
+    """A file within Synapse.
+
+    Attributes:


Due to microsoft/pylance-release#4759 pylance does not support seeing attributes in the doc string and linking them in the hover document. You'll see in these files that the information is duplicated.

Ahhh, I see. That's really unfortunate...

vpchung · 2023-12-12T00:28:58Z

mkdocs.yml

+  palette:
+    - media: "(prefers-color-scheme: light)"
+      scheme: default
+      primary: teal


It's hard to see the Synapse logo using this color:

I tested some other colors, and these work a little better:

white

black

cyan (though the green part of the Sage logo gets a little lost)

yellow - but I consider this chaotic good 🙃

We can also move forward with a custom color too.

I think white for light theme and black for dark theme look pretty decent:

vpchung · 2023-12-12T00:29:44Z

mkdocs.yml

+        name: Switch to dark mode
+    - media: "(prefers-color-scheme: dark)"
+      scheme: slate
+      primary: deep purple


Same note as above:

vpchung · 2023-12-12T00:32:53Z

setup.cfg

+    mkdocs-material>=9.4.14
+    mkdocstrings>=0.24.0
+    mkdocstrings-python>=1.7.5
+    termynal>=0.11.1


Feel free to remove termynal from the build if it's not utilized.

It's fun, though 😄

That does look fun, that would add some dynamic-ness to our docs.

I think I do want to leave it in. I'm not using it for anything yet, but it'll be nice to show for things like file transfers.

vpchung

This is awesome, thanks!!!

My only nitpick is with the color palette used for the navbar.

thomasyu888 · 2023-12-12T16:47:07Z

synapseclient/models/project.py

+
+    Tip: Title for my tip.
+        Testing what a tip looks like.
+
+    Todo:
+        * Testing what a todo looks like.
+        * Testing what a todo looks like.
+
+    Deprecated:
+        Testing what a deprecated section looks like.
+
+    I can put anything here for the title of a section:
+        askldjfghasdkljfghasdklfhasd;ofgyadilfghadfklhgdklghdsklfghksdhfaglkh
+
+    Raises:
+        Exception: Testing what an exception looks like.
+
+    Yields:
+        Testing what a yield section looks like.
+


We can clean this up at a later date

thomasyu888

💯 LGTM! I added some comments, but I'm very pleased with this.

What we will need to do is be diligent about some of the potential broken links to the python client docs from the synapse help docs once we push to master.

BryanFauble · 2023-12-12T16:59:46Z

What we will need to do is be diligent about some of the potential broken links to the python client docs from the synapse help docs once we push to master.

Thanks @thomasyu888 . My plan is to make a branch from develop and bring forward the minimum number of changes required to have a 1-to-1 (Or close to) transition from the current python client docs into this format. That way there are less things to review and go wrong. Once we are in the format than we have a great base to start adding all of these additional documents.

pep8speaks · 2023-12-12T20:14:37Z

Hello @BryanFauble! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file synapseclient/client.py:

Line 211:89: E501 line too long (99 > 88 characters)

In the file synapseclient/models/file.py:

Line 27:89: E501 line too long (92 > 88 characters)
Line 30:89: E501 line too long (92 > 88 characters)
Line 35:89: E501 line too long (90 > 88 characters)
Line 36:89: E501 line too long (97 > 88 characters)
Line 42:89: E501 line too long (101 > 88 characters)
Line 43:89: E501 line too long (91 > 88 characters)
Line 48:89: E501 line too long (103 > 88 characters)
Line 50:89: E501 line too long (96 > 88 characters)
Line 52:89: E501 line too long (94 > 88 characters)
Line 161:89: E501 line too long (110 > 88 characters)
Line 221:89: E501 line too long (110 > 88 characters)
Line 246:89: E501 line too long (110 > 88 characters)

In the file synapseclient/models/folder.py:

Line 26:89: E501 line too long (94 > 88 characters)
Line 27:89: E501 line too long (93 > 88 characters)
Line 31:89: E501 line too long (97 > 88 characters)
Line 32:89: E501 line too long (97 > 88 characters)
Line 40:89: E501 line too long (96 > 88 characters)
Line 42:89: E501 line too long (94 > 88 characters)
Line 128:89: E501 line too long (110 > 88 characters)
Line 209:89: E501 line too long (110 > 88 characters)
Line 270:89: E501 line too long (110 > 88 characters)

In the file synapseclient/models/project.py:

Line 24:89: E501 line too long (95 > 88 characters)
Line 25:89: E501 line too long (97 > 88 characters)
Line 29:89: E501 line too long (94 > 88 characters)
Line 30:89: E501 line too long (101 > 88 characters)
Line 38:89: E501 line too long (96 > 88 characters)
Line 39:89: E501 line too long (95 > 88 characters)
Line 40:89: E501 line too long (106 > 88 characters)
Line 118:89: E501 line too long (92 > 88 characters)
Line 175:89: E501 line too long (110 > 88 characters)
Line 251:89: E501 line too long (110 > 88 characters)
Line 312:89: E501 line too long (110 > 88 characters)

In the file synapseclient/models/table.py:

Line 340:89: E501 line too long (89 > 88 characters)
Line 346:89: E501 line too long (90 > 88 characters)
Line 347:89: E501 line too long (97 > 88 characters)
Line 350:89: E501 line too long (95 > 88 characters)
Line 356:89: E501 line too long (98 > 88 characters)
Line 359:89: E501 line too long (95 > 88 characters)
Line 361:89: E501 line too long (94 > 88 characters)
Line 463:89: E501 line too long (110 > 88 characters)
Line 489:89: E501 line too long (110 > 88 characters)
Line 514:89: E501 line too long (110 > 88 characters)
Line 593:89: E501 line too long (110 > 88 characters)
Line 617:89: E501 line too long (110 > 88 characters)
Line 644:89: E501 line too long (110 > 88 characters)

BryanFauble added 2 commits December 11, 2023 15:17

Migrate to mkdocstrings

1052438

Add back html build docs

b0bc07a

BryanFauble requested a review from a team as a code owner December 11, 2023 22:32

BryanFauble commented Dec 11, 2023

View reviewed changes

Revert pipfile

fb4ca3a

Sage-Bionetworks deleted a comment from pep8speaks Dec 11, 2023

vpchung reviewed Dec 12, 2023

View reviewed changes

vpchung suggested changes Dec 12, 2023

View reviewed changes

thomasyu888 reviewed Dec 12, 2023

View reviewed changes

thomasyu888 approved these changes Dec 12, 2023

View reviewed changes

Changing color scheme and adding nav tabs

b39bf49

BryanFauble merged commit 3aec4ff into SYNPY-1322-OOP-POC Dec 12, 2023
30 checks passed

BryanFauble deleted the SYNPY-1345-documentation-refresh branch December 12, 2023 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYNPY-1345] Migrate to mkdocstrings #1023

[SYNPY-1345] Migrate to mkdocstrings #1023

BryanFauble commented Dec 11, 2023 •

edited

BryanFauble Dec 11, 2023

vpchung Dec 12, 2023

BryanFauble Dec 12, 2023

BryanFauble Dec 11, 2023

thomasyu888 Dec 12, 2023 •

edited

BryanFauble Dec 11, 2023

thomasyu888 Dec 12, 2023

vpchung Dec 12, 2023

BryanFauble Dec 12, 2023

vpchung Dec 12, 2023

vpchung Dec 12, 2023

thomasyu888 Dec 12, 2023

BryanFauble Dec 12, 2023

vpchung left a comment

thomasyu888 Dec 12, 2023

thomasyu888 left a comment •

edited

BryanFauble commented Dec 12, 2023

pep8speaks commented Dec 12, 2023

[SYNPY-1345] Migrate to mkdocstrings #1023

[SYNPY-1345] Migrate to mkdocstrings #1023

Conversation

BryanFauble commented Dec 11, 2023 • edited

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

thomasyu888 Dec 12, 2023 • edited

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

vpchung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasyu888 left a comment • edited

Choose a reason for hiding this comment

BryanFauble commented Dec 12, 2023

pep8speaks commented Dec 12, 2023

BryanFauble commented Dec 11, 2023 •

edited

thomasyu888 Dec 12, 2023 •

edited

thomasyu888 left a comment •

edited