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 documentation #1474

Merged
merged 1 commit into from
Oct 9, 2022
Merged

New documentation #1474

merged 1 commit into from
Oct 9, 2022

Conversation

jules-ch
Copy link
Collaborator

@jules-ch jules-ch commented Feb 23, 2022

This my go to rework the Pint Documentation.

About the theme, we can choose between, they are widely used & offer good reading experience :

For the pydata sphinx theme I need to rearrange the toc tree a bit to have a proper Userguide, API Reference, dev guide if we choose it.

@jules-ch
Copy link
Collaborator Author

@hgrecco you can build it and tell me what you think.
i've changed some directive to ipython to have better syntax hightlighting & restructured a bit.
You can change the theme to furo by installing it and put html_theme = 'furo' to see this one.

@keewis
Copy link
Contributor

keewis commented Feb 23, 2022

I'd also recommend using autosummary to document the API: it creates overview tables that are much easier to read than the output of autodoc directives

@jules-ch
Copy link
Collaborator Author

I'd also recommend using autosummary to API: it creates overview pages that are much easier to read than the output of autodoc directives

Thanks for the tip

@hgrecco
Copy link
Owner

hgrecco commented Feb 24, 2022

@jules-ch I like it a lot. It is clean, with good contrast, readability and tables and code are properly colored. Go for it.

A few things that I have noticed:

  1. The note in the initial page ("One Last thing") is rendered to big.
  2. In FAQ, the list of other packages is not rendered as a list
  3. You can go to the "sections" (Getting started, User Guide, Advanced topics, ...) but nothing or almost nothing is there. It is weird.
  4. Instead of putting stackoverflow, why don't we create a discussion foum within github. This will also serve to remove questions "how do I" from the issue tracker.
  5. It would be good to split up the developer reference.

docs/user/index.rst Outdated Show resolved Hide resolved
@jules-ch
Copy link
Collaborator Author

jules-ch commented Mar 2, 2022

@hgrecco , I'll try to finish & merge new docs next week.
After that I think we can release a new version.

Then code reorganization will follow. What do you think @hgrecco

@hgrecco
Copy link
Owner

hgrecco commented Mar 2, 2022

Sounds Perfect!

@hgrecco
Copy link
Owner

hgrecco commented Mar 2, 2022

I am already working in the code reorganization (I will rebase afterwards) just to get a flavor of how it would look like.

@jules-ch
Copy link
Collaborator Author

jules-ch commented Mar 2, 2022

Yeah saw that, just wanted to know how you would like to proceed since it will have an impact on API reference mostly.

@keewis keewis mentioned this pull request Mar 2, 2022
21 tasks
@keewis
Copy link
Contributor

keewis commented Mar 16, 2022

ping, @jules-ch. Is there anything I can do to help finishing this? Otherwise, should we release now and include the new documentation in the next release? There's a few bug fixes that would be really important to be included in a release very soon.

@jules-ch
Copy link
Collaborator Author

jules-ch commented May 4, 2022

Gonna push this next week for the next release at least the new trees & theme & start from there with new contents.

@keewis
Copy link
Contributor

keewis commented Aug 23, 2022

@jules-ch, any updates on this? I think this was close enough, and the only thing left to do is to fix the linter job.

@jules-ch
Copy link
Collaborator Author

With the reorganization in facets, I need to update conflicts & API reference.
I haven't had much time lately, I'll try to look into it soon.

@keewis
Copy link
Contributor

keewis commented Aug 23, 2022

I'll try to look into it soon.

sure, no pressure.

FYI the merge conflicts seem to be pretty small (I'd think the only one caused directly by the facets is two lines in the numpy notebook), and I'd estimate you'd need about 10-15 minutes at most to resolve them.

@jules-ch
Copy link
Collaborator Author

jules-ch commented Sep 3, 2022

Ok this is ready to be merged.

There is still work to be done to have a proper API Reference that we can link in the documentation.

@jules-ch jules-ch changed the title Draft: New documentation New documentation Sep 3, 2022
Copy link
Contributor

@keewis keewis left a comment

Choose a reason for hiding this comment

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

I have a few questions, but otherwise this looks good to me, and I agree that anything further (like the API docs) can be done in a separate PR

docs/user/performance.rst Show resolved Hide resolved
docs/user/performance.rst Show resolved Hide resolved
docs/user/performance.rst Outdated Show resolved Hide resolved
@jules-ch jules-ch force-pushed the new-documentation branch 2 times, most recently from a2cc663 to e0932e0 Compare October 4, 2022 21:30
@jules-ch
Copy link
Collaborator Author

jules-ch commented Oct 5, 2022

@keewis & @hgrecco if you can make a final review.
API reference is somewhat difficult to address wit the new facets split.
I'll see what I can do after this merge.

I tried to stick to the style of numpy, pandas etc.
They tried to standardize how documentation is splitted.

tell me what you think & I will merge this

https://pint--1474.org.readthedocs.build/en/1474/

@hgrecco
Copy link
Owner

hgrecco commented Oct 5, 2022

Congratulation for all the awesome work! It looks fantastic. The facets look fine to me ,but I am maybe not the best person to judge as I know perfectly where everything is and why! In any case, I like it a lot.

My only suggestion is if there is room for a code snippet in the main page. What do you think? I usually like to see how it looks at a glance!

@andrewgsavage
Copy link
Collaborator

The links on https://pint--1474.org.readthedocs.build/en/1474/user/numpy.html#Technical-Commentary are slightly messed up now.

Is there a way to disable intpretting the unit definition code blocks on https://pint--1474.org.readthedocs.build/en/1474/user/defining.html as python code? Seeing 60 and min colored looks off.

To me it seems odd to see 3 topics under "Going further", I think they would fit under advanced topics. Not a big deal though.

There are two pint.facets.numpy in the API reference

@hgrecco
Copy link
Owner

hgrecco commented Oct 6, 2022

One more thing (sorry) that I just noticed. At the footer I would rather put By the Pint Authors (and if possible a link to the AUTHORS file).

And in Contributing: We do not use travis anymore.

@jules-ch
Copy link
Collaborator Author

jules-ch commented Oct 6, 2022

One more thing (sorry) that I just noticed. At the footer I would rather put By the Pint Authors (and if possible a link to the AUTHORS file).

Ok, I'll make the changes

And in Contributing: We do not use travis anymore.

Yep saw that commit haven't been pushed yesterday xD

@keewis
Copy link
Contributor

keewis commented Oct 6, 2022

From a quick glance, this looks good to me. I'd probably fix the the footer and merge afterwards: this change is big enough already, so I think anything further would be easier to review / merge in separate PRs.

The links in the numpy technical commentary have been broken for quite some time (this is a known limitation of nbsphinx), so while I think that we definitely should fix them, I don't think that has to be in this PR. Same with the syntax highlighting of the unit definitions (I think the reason is the use of :: to create code blocks, explicitly using .. code:: might get rid of that?).

As for the facets: those are pretty advanced and most likely only contributors would read it, but I think we should try to create a new page describing the API to help new contributors with understanding the general structure of the code base (but once again, I don't think that should be done in this PR).

@jules-ch
Copy link
Collaborator Author

jules-ch commented Oct 9, 2022

I agree with @keewis. I'll try to fix most of your remarks but we need to get this out & work form there.
I've delayed this for quite some time now :D

- Rework of configuration with new sphinx_book_theme
- Use ipython directive where necessary
- Add theme to requirements_docs.txt
- Start using autosummarry
- Fix doctests
- Add sphinx design to requirements
- Add overview & panels in home page
- Update footer with `Pint Developers`
- Fix docstrings
- Use string imports in __all__ in __init__.py
@jules-ch
Copy link
Collaborator Author

jules-ch commented Oct 9, 2022

I've changed the footer.

I'll wait for CI to finish & I'm gonna merge this. We can have follow up issues/PR.

@jules-ch jules-ch merged commit 052a920 into hgrecco:master Oct 9, 2022
@jules-ch jules-ch mentioned this pull request Oct 10, 2022
@dcamron dcamron mentioned this pull request Oct 26, 2022
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.

4 participants