-
Notifications
You must be signed in to change notification settings - Fork 7.1k
EfficientNet New Documentation #5854
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @yassineAlouini , minor comment but this looks great!
By the way, I thought about being clever with the docstrings
I started with a similar idea as well, but because this makes us document the model builders in slightly unconventional ways (the docstring wouldn't be right below the thing it's documenting), we decided against it, at least for now
Awesome, I will fix these small details this afternoon and start the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @yassineAlouini , LGTM!
Thanks @NicolasHug. |
Summary: * [DOC] Add new EfficientNet doc. * [DOC] Add missing file. * [FIX] Precommits fix. * [FIX] Precommits fix. * [ENH] Integrate few documentation comments from code review * Update docs/source/models/efficientnet.rst Reviewed By: jdsgomes, NicolasHug Differential Revision: D36095667 fbshipit-source-id: 5f14099067480da795edb7418d7a4e2232e8c300 Co-authored-by: Nicolas Hug <[email protected]>
In this PR, I add the EfficientNet documentation following the guidelines outlined here.
By the way, I thought about being clever with the docstrings (since they repeat between the different) but then refrained from being clever (better explicit than implicit etc). Here is the code I had in mind nonetheless:
efficientnet_b0.__doc__ = EFFICIENTNET_DOCSTRING.format(submodel="bo")