-
Notifications
You must be signed in to change notification settings - Fork 85
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: Drop modifierclass and improve autosummary #1338
Conversation
Notes to self:
|
Codecov Report
@@ Coverage Diff @@
## master #1338 +/- ##
=======================================
Coverage 97.50% 97.50%
=======================================
Files 63 63
Lines 3760 3760
Branches 538 538
=======================================
Hits 3666 3666
Misses 55 55
Partials 39 39
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
@kratsg overall I really like these changes. They make things much nicer IMO and I really like in general how the Infer docs are sectioned out now.
I'm not sure if it can be fixed here, but the "Minimizer Options" aren't being displayed correctly for the optimizer _minimize
methods.
we've never been able to see the _minimize
methods before, so it isn't surprising they might not look perfect.
What is the problem with the GitHub links? I think I missed this. |
the |
I had to go look and see what these buttons even were...so I think that's a very safe bet. |
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.
@kratsg I'll approve this now and I'll let you decide if we worry about the "Minimizer Options" render here or make a new Issue for it for another later PR.
Do you know how to fix the render? Particularly, it should just be specifying the additional keyword arguments that go into |
#1339 will handle the fix of the docstring. |
@kratsg Can you take a look at what I have now? I turned them into bulleted lists. |
Oh. Seems we just missed each other. Do you want me to revert this and make a new PR? |
Looks good. We'll close both issues in this PR and move on. |
okay sounds good. Let me push one more style fix. |
Pull Request Description
We've been a bit held back by the terrible
modifierclass.rst
I wrote a long time ago and it's been making some of the documentation a bit worse for the wear in general. This PR will migrate some portion of this into the autosummary extension instead underclass.rst
to only impact theautoclass
parts.Fixes #761, fixes #1339.
Checklist Before Requesting Reviewer
Before Merging
For the PR Assignees: