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

[docs] Rework documentation [new style / features / structure] #851

Closed
wants to merge 27 commits into from

Conversation

felixdittrich92
Copy link
Contributor

@felixdittrich92 felixdittrich92 commented Mar 10, 2022

This PR updates the current documentation

  • new more modern style (furo)
  • dark & light theme / framework split docu
  • restructure documentation
  • move code of cunduct & contributing to docu
  • add sample image grid for each dataset

motivation:
Every active modern library also needs decent documentation, which is the showpiece. Therefore we should improve it (even if it's not fun 😅) and try to cover everything as best as possible, including the framework difference

further todo: (maybe in another PR)

  • same image additions for transformations
  • read the docs grammar stuff and so on

issues:
#729 could be closed with this

Any feedback is very welcome 🤗
@fg-mindee @charlesmindee @SiddhantBahuguna wdyt ? 😄

Examples:
light
dark
light2
contrib

I have also checked how it looks as docstring while coding with the tabs and personally i found it pretty nice to have both snippets at once. The only drawback is the much larger docstring.

@codecov
Copy link

codecov bot commented Mar 10, 2022

Codecov Report

Merging #851 (d30902a) into main (096eb48) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #851   +/-   ##
=======================================
  Coverage   94.84%   94.84%           
=======================================
  Files         133      133           
  Lines        5200     5200           
=======================================
  Hits         4932     4932           
  Misses        268      268           
Flag Coverage Δ
unittests 94.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/datasets/cord.py 97.43% <ø> (ø)
doctr/datasets/detection.py 95.65% <ø> (ø)
doctr/datasets/doc_artefacts.py 93.93% <ø> (ø)
doctr/datasets/funsd.py 96.96% <ø> (ø)
doctr/datasets/generator/pytorch.py 100.00% <ø> (ø)
doctr/datasets/generator/tensorflow.py 100.00% <ø> (ø)
doctr/datasets/ic03.py 97.36% <ø> (ø)
doctr/datasets/ic13.py 96.15% <ø> (ø)
doctr/datasets/iiit5k.py 96.77% <ø> (ø)
doctr/datasets/imgur5k.py 92.50% <ø> (ø)
... and 45 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 096eb48...d30902a. Read the comment docs.

@felixdittrich92 felixdittrich92 changed the title [WIP] [docs] Rework documentation [new style / features / structure] [docs] Rework documentation [new style / features / structure] Mar 11, 2022
@felixdittrich92 felixdittrich92 marked this pull request as ready for review March 11, 2022 10:46
@fg-mindee
Copy link
Contributor

That's a very interesting PR :)

I have to admit, it would have been nice to split it in a few haha

General comments first:

  • the code of conduct & contributing markdown cannot be moved (it's for github). But if you need them someplace else, we can create a symlink
  • could you explain the new dependencies of the documentation please?
  • for the dataset images, I'll add them in a release attachments to limit the amount of content files in the repo
  • I see that you use tabs for PyTorch/TensorFlow: it's really neat, I'd love to have that. However, I didn't see how you mean to build this for both framework (you can't build for both of them at once)

@fg-mindee fg-mindee self-assigned this Mar 11, 2022
@fg-mindee fg-mindee added topic: documentation Improvements or additions to documentation type: enhancement Improvement labels Mar 11, 2022
@fg-mindee fg-mindee self-requested a review March 11, 2022 18:36
@felixdittrich92
Copy link
Contributor Author

@fg-mindee
Yes i know but the most stuff in this PR depends on other so really hard to minimize 😅

  1. I agree but i would say we hold this also in the documentation so we have this twice but can link in the readme directly to docs. Wdyt?
  2. furo is a theme like the old sphinx_rtd_theme and sphinx-tabs is an sphinx extension which i have used to realize the tf/pytorch Code example split :)
  3. 👍 than we need only change to the url
  4. Yes i know i have take care of this directly in the docstrings you can check this in any changed model file (for example resnet) :D

@felixdittrich92
Copy link
Contributor Author

felixdittrich92 commented Mar 11, 2022

@fg-mindee
Point 1: applied as symlinks and reverted both files 👍

Point 3: zip file with dataset image-grids attatched
dataset_image_grid_examples.zip

Point 4:
Every object which shares the same namespace in TF and PT implementation (and is also implemented same : which makes the transform part a bit more tricky) has now a shared docstring (same in TF and PT implementation).
Hopefully this makes a bit more clear how it is possible that we have this tab seperated code examples 🤗

I have added es for both TF implementation and PT. Why? :

  1. if we want we could easy switch "as backend" to pt for the docs without any loss
  2. consistency !
    I have tested this docstrings also while coding and its really nice to have both samples in one way i like it 😅
    Disadvantage are a bit longer docstrings
.. tabs::

        .. tab:: TensorFlow

            .. code:: python

                >>> import tensorflow as tf
                >>> from doctr.models import vgg16_bn_r
                >>> model = vgg16_bn_r(pretrained=False)
                >>> input_tensor = tf.random.uniform(shape=[1, 512, 512, 3], maxval=1, dtype=tf.float32)
                >>> out = model(input_tensor)

        .. tab:: PyTorch

            .. code:: python

                >>> import torch
                >>> from doctr.models import vgg16_bn_r
                >>> model = vgg16_bn_r(pretrained=False)
                >>> input_tensor = torch.rand((1, 3, 512, 512), dtype=torch.float32)
                >>> out = model(input_tensor)

1
2

@fg-mindee
Copy link
Contributor

Would it be possible to split this into several PRs please? 🙏

  • one for the change of theme + JS/CSS
  • updating the docstrings of datasets with images
  • the rest in a final PR

That would help reducing the amount of files modified here 😅

Copy link
Contributor

@fharper fharper left a comment

Choose a reason for hiding this comment

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

LGTM

@fg-mindee fg-mindee mentioned this pull request Mar 18, 2022
85 tasks
@fg-mindee
Copy link
Contributor

Now that all the subPRs have been merged, this PR can be closed :)
Thanks a lot for your time @felixdittrich92 🙏 This is a huge improvement to the general look of the documentation!!

@fg-mindee fg-mindee closed this Mar 18, 2022
@felixdittrich92 felixdittrich92 deleted the docs-improvement branch March 18, 2022 15:29
@fharper
Copy link
Contributor

fharper commented Mar 18, 2022

@felixdittrich92: thanks for this update, I really love the new visual!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Improvements or additions to documentation type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants