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

New theme for our documentation #159

Merged
merged 3 commits into from
Oct 23, 2024
Merged

New theme for our documentation #159

merged 3 commits into from
Oct 23, 2024

Conversation

JOJ0
Copy link
Contributor

@JOJ0 JOJ0 commented Oct 12, 2024

Hi @AnssiAhola and @prcutler,
in the past we put a lot of effort in our documentation. We also have quite a lot of content and I think we deserve a modern looking and feature-rich layout.

What do you think of changing our docs theme to the PyData theme? Since I did this elsewhere I learned some customization possibilities. One default feature of PyData that I think fits our docs, since sometimes we have rather long main chapters, is the "secondary navigation" that helps to easily switch chapters from within one main chapter.

If you don't like this theme at all, I can certainly apply another one and we re-evaluate. I enable this PR branch on readthedocs so we get a live view.

https://python3-discogs-client.readthedocs.io/en/docs_theme/index.html

@JOJ0
Copy link
Contributor Author

JOJ0 commented Oct 12, 2024

One thing that I don't understand is how and why the left bars "Section navigation" is always empty. Maybe some restructuring of chapters/content would be required to make use of that. Everything seems to be on the very right "On this page" navigation. See for comparision in the beets.io docs, "Section Navigation" is populated, I need to investigate why it works there.... https://beets.readthedocs.io/en/stable/reference/config.html

@AnssiAhola
Copy link
Contributor

@JOJ0 Looks cool, I'm all for it!

@prcutler
Copy link
Contributor

I like it. I looked at the PyData Sphinx page, too, which I liked. It made me think of re-organizing our content a bit if we move to that theme, but one thing at a time. 😀

@JOJ0
Copy link
Contributor Author

JOJ0 commented Oct 13, 2024

Unfortunately there is one thing that is worse with this theme and I don't know how to fix it.

Vertical spacing here seems off - it's "way too much" in my opinion:

Screenshot 2024-10-13 at 16 03 59

With the readthedocs theme it didn't use that much space

Screenshot 2024-10-13 at 16 01 20

Can we live with that?

@JOJ0
Copy link
Contributor Author

JOJ0 commented Oct 13, 2024

One thing I changed "content wise", is restructure the Python docs by inventing another level of index. See 0cd0b2a for details

Basically it separates the huge "one page" Python API documentation into single pages and let's us switch with the "Section Navigation". I think this is much more "navigation friendly" now. So now a combination of left and right sidebars can be use to navigate more comfortably even though it's rather a lot of content:

Screenshot 2024-10-13 at 16 14 16

@JOJ0
Copy link
Contributor Author

JOJ0 commented Oct 13, 2024

I like it. I looked at the PyData Sphinx page, too, which I liked. It made me think of re-organizing our content a bit if we move to that theme, but one thing at a time. 😀

I agree that re-organizing could further improve the "navigateabilty" (that a word? 😆 ). I'm happy to help and review if you want to work on it. One thing I just found out is how to do "subindexes" which helps structuring better. See the changes I did in above mentioned commit. It should get you started for this type of restructuring.

@JOJ0
Copy link
Contributor Author

JOJ0 commented Oct 19, 2024

Unfortunately there is one thing that is worse with this theme and I don't know how to fix it.

Vertical spacing here seems off - it's "way too much" in my opinion:

Screenshot 2024-10-13 at 16 03 59

With the readthedocs theme it didn't use that much space

Screenshot 2024-10-13 at 16 01 20

Can we live with that?

I can. Not a showstopper for me. You guys?

Showstopper for me right now is a lot of new warnings. Will post this weekend hopefully.

@prcutler
Copy link
Contributor

Unfortunately there is one thing that is worse with this theme and I don't know how to fix it.
Vertical spacing here seems off - it's "way too much" in my opinion:

Can we live with that?

I can. Not a showstopper for me. You guys?

Showstopper for me right now is a lot of new warnings. Will post this weekend hopefully.

Sorry for the late response - that's not a showstopper for me, either. I'm a bit busy this month, but I should have some time in a couple weeks to work on the docs.

@JOJ0
Copy link
Contributor Author

JOJ0 commented Oct 23, 2024

Unfortunately there is one thing that is worse with this theme and I don't know how to fix it.
Vertical spacing here seems off - it's "way too much" in my opinion:

Can we live with that?

I can. Not a showstopper for me. You guys?
Showstopper for me right now is a lot of new warnings. Will post this weekend hopefully.

Sorry for the late response - that's not a showstopper for me, either. I'm a bit busy this month, but I should have some time in a couple weeks to work on the docs.

No worries, sounds perfect, whenever you find the time, or not. Our docs are not bad at all right now! So no stress in there! :-)

Thanks for your opinion, on that "too much spacing issue". Yeah let's move on. When there is an actual comment on that specific property, it does look ok by the way:

Screenshot 2024-10-23 at 07 24 23

Regarding the sh**load of warnings I was getting....I can't reproduce anymore and I'll leave it be for now and merge this :-)

Just for reference, and if you come across something like it next time you work on the docs, I got a lot of WARNING: duplicate object description of...:

reading sources... [100%] writing_docs
/Users/jtiefenbacher/git/discogs_client/discogs_client/models.py:docstring of discogs_client.models.SimpleFieldDescriptor:10: ERROR: Unexpected indentation. [docutils]
/Users/jtiefenbacher/git/discogs_client/discogs_client/models.py:docstring of discogs_client.models.ObjectFieldDescriptor:14: ERROR: Unexpected indentation. [docutils]
/Users/jtiefenbacher/git/discogs_client/discogs_client/models.py:docstring of discogs_client.models.ListFieldDescriptor:8: ERROR: Unexpected indentation. [docutils]
/Users/jtiefenbacher/git/discogs_client/discogs_client/models.py:docstring of discogs_client.models.ObjectCollectionDescriptor:9: ERROR: Unexpected indentation. [docutils]
/Users/jtiefenbacher/git/discogs_client/discogs_client/client.py:docstring of discogs_client.client:1: WARNING: duplicate object description of discogs_client.client, other instance in discogs_client, use :no-index: for one of them
/Users/jtiefenbacher/git/discogs_client/discogs_client/client.py:docstring of discogs_client.client.Client:1: WARNING: duplicate object description of discogs_client.client.Client, other instance in discogs_client, use :no-index: for one of them
/Users/jtiefenbacher/git/discogs_client/discogs_client/client.py:docstring of discogs_client.client.Client._base_url:1: WARNING: duplicate object description of discogs_client.client.Client._base_url, other instance in discogs_client, use :no-index: for one of them
/Users/jtiefenbacher/git/discogs_client/discogs_client/client.py:docstring of discogs_client.client.Client._request_token_url:1: WARNING: duplicate object description of discogs_client.client.Client._request_token_url, other instance in discogs_client, use :no-index: for one of them
...
...

I'll spare us the whole output, it's repeating that line for all kinds of code parts. Also note: The first 4 lines I just realized are "expected", we have them in a "master" build as well, and live with them since ever (Unexpeced Indentation ...).

Just a reminder that you will most probably get them when working on the docs next time :-)

Ok, let's merge.

Johannes Tiefenbacher added 3 commits October 23, 2024 07:34
- This enables "Section Navigation"
- Each module has it's own .rst file with a "main section headline",
- and is bound together by a "subindex" defined by
  index_discogs_client.rst
@JOJ0 JOJ0 merged commit b73595d into master Oct 23, 2024
10 checks passed
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.

3 participants