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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback docs with autosummary #3908

Merged
merged 3 commits into from Oct 6, 2020
Merged

Callback docs with autosummary #3908

merged 3 commits into from Oct 6, 2020

Conversation

ydcjeff
Copy link
Contributor

@ydcjeff ydcjeff commented Oct 6, 2020

What does this PR do?

Fixes #2034

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 馃檭

@williamFalcon @Borda @awaelchli @edenlightning

@mergify mergify bot requested a review from a team October 6, 2020 16:50
@@ -40,6 +40,7 @@

class EarlyStopping(Callback):
r"""
Monitor a validation metric and stop training when it stops improving.
Copy link
Contributor Author

@ydcjeff ydcjeff Oct 6, 2020

Choose a reason for hiding this comment

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

Adding this to appear the docstring in the left side of table row.
e.g: https://pytorch.org/docs/stable/nn.html#containers

Comment on lines +1 to +3
col {
width: 50% !important;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is needed as there is colgroup html tag which apply width 10% to the right and 90% to the left.
Not sure where does first appear, can't inspect that in pytorch docs.
So manually setting it.

Comment on lines +1 to +14
.. role:: hidden
:class: hidden-section
.. currentmodule:: {{ module }}


{{ name | underline }}

.. autoclass:: {{ name }}
:members:


..
autogenerated from source/_templates/classtemplate.rst
note it does not have :inherited-members:
Copy link
Contributor Author

@ydcjeff ydcjeff Oct 6, 2020

Choose a reason for hiding this comment

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

This is the template for autoclass to appear like pytorch docs.
without this, the h1 will be module.{module}.class, which is long for sub packages.

EDIT: copied from pytorch docs repo

@ydcjeff
Copy link
Contributor Author

ydcjeff commented Oct 6, 2020

Shall we show private methods?

@codecov
Copy link

codecov bot commented Oct 6, 2020

Codecov Report

Merging #3908 into master will increase coverage by 2%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #3908    +/-   ##
=======================================
+ Coverage      83%     86%    +2%     
=======================================
  Files         117     117            
  Lines        9237    9471   +234     
=======================================
+ Hits         7689    8117   +428     
+ Misses       1548    1354   -194     

@Borda Borda added docs Documentation related feature Is an improvement or enhancement labels Oct 6, 2020
@ydcjeff
Copy link
Contributor Author

ydcjeff commented Oct 6, 2020

Changes here: https://61555-178626720-gh.circle-artifacts.com/0/html/callbacks.html#built-in-callbacks

In ModelCheckpoint or some other like, referencing to other classes, class's params will not show the link as the referencing link for those isn't include in this PR as api docs deleted and to keep PR reviewable.

Created Trainer docs with autosummary locally and referencing works as before.

In the latest docs, those ref links point to api docs.

@williamFalcon williamFalcon merged commit fe5b943 into Lightning-AI:master Oct 6, 2020
@Borda
Copy link
Member

Borda commented Oct 6, 2020

very nice :]
@ydcjeff mind check if we can with this new plugin export methods to be visible/listed on the right side?
basically, it means that the method has to be some kind of heading...

@Borda
Copy link
Member

Borda commented Oct 6, 2020

@ydcjeff regarding your this docs update, is this is relevant? #3503
because now I see that links to source work fine...

awaelchli pushed a commit that referenced this pull request Oct 6, 2020
* callback docs with autosummary

* do not show private methods

* callback base docstring
@ydcjeff
Copy link
Contributor Author

ydcjeff commented Oct 7, 2020

very nice :]
@ydcjeff mind check if we can with this new plugin export methods to be visible/listed on the right side?
basically, it means that the method has to be some kind of heading...

Unfortunately, autosummary exports the class with autoclass, function with autofunction

So, appending heading is kinda hard.
I initially thought abt this, but couldn't figure out how to do. will find a way.

@ydcjeff
Copy link
Contributor Author

ydcjeff commented Oct 7, 2020

@ydcjeff regarding your this docs update, is this is relevant? #3503
because now I see that links to source work fine...

What the PR does linking the source code to the docs from _modules folder.

For methods like training_step in LightningModule docs can be solved with automethod.

It can be said that PR is relevant or not relevant.

The core team needs to decide source will link to github or link to _modules folder.

williamFalcon added a commit that referenced this pull request Oct 7, 2020
* base

* add xfail

* new test

* import

* missing import

* xfail if not installed


include mkpatch


fix test

* mock comet


comet mocks


fix test


remove dep


undo merge duplication

* line

* line

* convert doctest

* doctest

* docs

* prune Results usage in notebooks (#3911)

* notebooks

* notebooks

* revamp entire metrics (#3868)

* removed metric

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* added new metrics

Co-authored-by: Teddy Koker teddy.koker@gmail.com

* pep8

Co-authored-by: Teddy Koker teddy.koker@gmail.com

* pep8

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* docs

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* docs

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* win ddp tests skip

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* win ddp tests skip

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* win ddp tests skip

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* win ddp tests skip

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* reset in compute, cache compute

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* reduce_ops handling

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* sync -> sync_dist, type annotations

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* wip docs

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* mean squared error

* docstring

* added mean ___ error metrics

* added mean ___ error metrics

* seperated files

* accuracy doctest

* gpu fix

* remove unnecessary mixin

* metric and accuracy docstring

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* metric docs

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* pep8, changelog

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* refactor dist utils, pep8

* refactor dist utils, pep8

Co-authored-by: Teddy Koker <teddy.koker@gmail.com>

* Callback docs with autosummary (#3908)

* callback docs with autosummary

* do not show private methods

* callback base docstring

* skip some docker builds (temporally pass) (#3913)

* skip some docker builds

* todos

* skip

* use badges only with push (#3914)

* testtube

* mock test tube

* mock mlflow

* remove mlflow

* clean up

* test

* test

* test

* test

* test

* test

* code blocks

* remove import

* codeblock

* logger

* wandb causes stall

Co-authored-by: William Falcon <waf2107@columbia.edu>
Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Co-authored-by: Ananya Harsh Jha <ananya@pytorchlightning.ai>
Co-authored-by: Teddy Koker <teddy.koker@gmail.com>
Co-authored-by: Jeff Yang <ydcjeff@outlook.com>
@ydcjeff ydcjeff deleted the docs/callback branch October 7, 2020 04:32
@ydcjeff ydcjeff mentioned this pull request Oct 7, 2020
7 tasks
@Borda Borda added this to the 0.10.0 milestone Oct 7, 2020
@pbmstrk pbmstrk mentioned this pull request Oct 9, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related feature Is an improvement or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs are missing the anchor links
3 participants