-
-
Notifications
You must be signed in to change notification settings - Fork 427
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
Add always-visible titles to the example gallery. #1484
Add always-visible titles to the example gallery. #1484
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.
I found the easter egg!
Is there a reason to not just use ex_title
as the title
? We could even start throwing exceptions or something if it wasn't found. I don't feel strongly either way.
@@ -3,6 +3,7 @@ | |||
==================== | |||
|
|||
_thumb: .5, .7 | |||
_example_title: Plot LOO overlayed on <i>some damn thing</i>. |
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.
watch your language!
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.
I was hoping you knew what it was overlayed on...
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.
...and whether it's "overlayed" or "overlaid"!
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.
overlaid i believe
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.
I would write something like Plot LOO-PIT KDE overlaid on uniform distributions KDEs
, but I find it quite hard to summarize that more that what is said in the Examples section of its API docs without loosing so much meaning it can only be understood if you already know what it means
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.
Can we assume someone using this would know what are CDF and inverse-CDF? Maybe Wikipedia's PIT article helps https://en.wikipedia.org/wiki/Probability_integral_transform ?
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.
@avehtari Probably we can make that assumption, in which case we can give this a title that explains it sufficiently for such a person.
Plot LOO overlaid on uniform distributions to assess convergence.
Would that be good?
This does beg the question -- if a person does not know the purpose of this plot, where should they look? API docs? Examples? Or do we need a tutorial of some form?
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.
I would say that ideally they would look at the API docs (each example already has a link to the relevant function) if necessary they can go to other resources from there, reference papers, it's page in the educational materials repo (which does not exist yet) and other relevant links that may be present.
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.
I put in the above suggested title -- "Plot LOO overlaid on uniform distributions to assess convergence." Marking this as resolved.
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.
- It's missing a word: Plot LOO "what" overlaid on...
LOO can mean many things, and the confusion could that some would think it's LOO elpds. If you think that probability integral transformation is not well known, how about "LOO predictive ECDF" ? - It's not about convergence, but about model checking. So the full sentence could be
"Plot LOO predictive ECDF overlaid on ECDFs of uniform samples to assess predictive calibration"
I put changes in
|
19773d2
to
18142db
Compare
Codecov Report
@@ Coverage Diff @@
## master #1484 +/- ##
=======================================
Coverage 91.92% 91.92%
=======================================
Files 105 105
Lines 11252 11252
=======================================
Hits 10343 10343
Misses 909 909
Continue to review full report at Codecov.
|
One thing that would be nice to fix before merging: separation plot.
|
I just pushed a new version with a better-centered matplotlib separation plot. |
Calling this good enough and clearing the WIP flag. |
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.
The only one I'm not sure about is the change to mpl_plot_ess_local
-- the reason I added the phrase that you took out was that the term "local" was not clear. What makes an expected sample size local?
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.
Other than that one point, these all seem like clear improvements.
These test failures aren't due to this MR. |
Last thing, if we have the energy, would be to apply these changes to the Bokeh figures, as well. |
The quantile ess is calculated with
reference: https://arxiv.org/pdf/1903.08008.pdf
I would wait for that, I want to try simplifying the example gallery using tabs like https://sphinx-panels.readthedocs.io/en/latest/#tabbed-content. After all, the title is the same, the api reference is the same, and it would allow having a single thumbnail and avoid generating bokeh thumbnails which is the main complication when generating the docs and requires weird dependencies. |
In that case, I think we are ready to merge. |
Previously, it was hard to find things in the gallery generator, because the only information was in tooltips that were hidden by default.
Extend the docstring based on explanation from Aki Vehtari
Improve the reference to make it easier for readers to find.
Embedded Latex causes complaints about anomalous strings from pydocstyle and pylint.
0ede2bd
to
4f13f4d
Compare
Previously the only titles were in tooltips, which made it near impossible to scan the page and find the example you wanted.
Note that this is a WIP if only because there's an Easter Egg in one of the titles that needs fixing!