Skip to content
This repository was archived by the owner on Jan 15, 2024. It is now read-only.

Conversation

leezu
Copy link
Contributor

@leezu leezu commented Jul 16, 2019

Description

This adds sphinx autodoc based on Python 3 typehints.

Impact: Auto-generate (missing) type annotations in the documentation based on machine-readable typehints. The auto-generated type annotations look as follows: http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-830/4/api/modules/vocab.html#gluonnlp.Vocab

@leezu leezu requested a review from szha as a code owner July 16, 2019 13:45
@leezu leezu force-pushed the autodoctypehints branch 3 times, most recently from 2464b49 to bb5fcdb Compare July 16, 2019 17:10
@mli
Copy link
Member

mli commented Jul 16, 2019

Found link check problems in job PR-830/4:
(line 19) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 5) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 5) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 21) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 442) broken https://www.aclweb.org/anthology/P02-1040.pdf)[1 - 404 Client Error: Not Found for url: https://www.aclweb.org/anthology/P02-1040.pdf)%5B1
(line 92) broken https://nlp.stanford.edu/pubs/glove.pdf)[2 - 404 Client Error: NOT FOUND for url: https://nlp.stanford.edu/pubs/glove.pdf)%5B2
(line 208) broken https://www.bioinf.jku.at/publications/older/2604.pdf)[3 - 404 Client Error: Not Found for url: https://www.bioinf.jku.at/publications/older/2604.pdf)%5B3

@mli
Copy link
Member

mli commented Jul 16, 2019

Job PR-830/4 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-830/4/index.html

@leezu
Copy link
Contributor Author

leezu commented Jul 16, 2019

@dmlc/gluon-nlp-team please take a look at above example and make your opinion known.

@JulianSlzr
Copy link

I think typing will add some automatic robustness and reduce unit test size. Some thoughts:

  • Can we make from typing import * the convention and implicitly reserve its subclass names (List, Optional, etc.)? If that's too permissive we can do from typing import Optional, List, .... Both would be preferable to wordy type hints param: typing.Optional[typing.List[int, typing...]]
  • How do we document tensor-shape constraints not in the type signature? We'd need a convention.
  • Idea: Could typing / NDArray be extended to support tensor shape checking? e.g., def f(...) -> NDArray.dim(3)

@leezu
Copy link
Contributor Author

leezu commented Jul 18, 2019

Thanks @JulianSlzr for your thoughts.

@leezu leezu mentioned this pull request Aug 1, 2019
5 tasks
@leezu leezu force-pushed the autodoctypehints branch from bb5fcdb to eb45759 Compare August 1, 2019 17:00
@leezu leezu changed the title [WIP] Autodoc typehints Autodoc typehints Aug 1, 2019
@codecov
Copy link

codecov bot commented Aug 1, 2019

Codecov Report

Merging #830 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #830      +/-   ##
==========================================
- Coverage   90.18%   90.17%   -0.01%     
==========================================
  Files          66       66              
  Lines        6345     6344       -1     
==========================================
- Hits         5722     5721       -1     
  Misses        623      623
Impacted Files Coverage Δ
src/gluonnlp/vocab/vocab.py 97.28% <100%> (-0.02%) ⬇️

@mli
Copy link
Member

mli commented Aug 1, 2019

Found link check problems in job PR-830/5:
(line 19) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 5) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 5) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 21) broken https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/ - 404 Client Error: Not Found for url: https://www.salesforce.com/products/einstein/ai-research/the-wikitext-dependency-language-modeling-dataset/
(line 442) broken https://www.aclweb.org/anthology/P02-1040.pdf)[1 - 404 Client Error: Not Found for url: https://www.aclweb.org/anthology/P02-1040.pdf)%5B1
(line 92) broken https://nlp.stanford.edu/pubs/glove.pdf)[2 - 404 Client Error: NOT FOUND for url: https://nlp.stanford.edu/pubs/glove.pdf)%5B2
(line 208) broken https://www.bioinf.jku.at/publications/older/2604.pdf)[3 - 404 Client Error: Not Found for url: https://www.bioinf.jku.at/publications/older/2604.pdf)%5B3

@mli
Copy link
Member

mli commented Aug 1, 2019

Job PR-830/5 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-830/5/index.html

Copy link
Member

@eric-haibin-lin eric-haibin-lin left a comment

Choose a reason for hiding this comment

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

Nice! I guess the next step is to apply these changes to all other API docs?

@mli
Copy link
Member

mli commented Aug 3, 2019

Job PR-830/6 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-830/6/index.html

@mli
Copy link
Member

mli commented Aug 3, 2019

Job PR-830/7 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-830/7/index.html

@leezu leezu merged commit f9e9b53 into dmlc:master Aug 3, 2019
@leezu leezu deleted the autodoctypehints branch August 3, 2019 11:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants