Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
DOC: initial tags for statistics section of gallery#27569
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
bcc5d03 tob7fcf69Compareksunden commentedJul 13, 2024
Cycling to rebuild docs, now that upstream has been updated |
QuLogic left a comment
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.
This also needs a rebase.
Uh oh!
There was an error while loading.Please reload this page.
galleries/examples/statistics/multiple_histograms_side_by_side.py OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
story645 commentedSep 13, 2024
Cleaned this one up, thanks! |
QuLogic commentedSep 14, 2024
The build is broken because of#28820, but fixing that, it'sreally broken because |
story645 commentedSep 15, 2024
how come? (Last I checked, we don't do any validation on tags) |
timhoffm commentedSep 16, 2024
I'd prefer that we don't use the tagging mechanism for internal suff. Making internal labels public is confusing, OTOH packing them behind It would work, if |
story645 commentedSep 16, 2024
That should if you put it behind a new tag directive |
timhoffm commentedSep 17, 2024
Well, I feel uncomfortable, with this duct-tape coding of internal tags via |
story645 commentedSep 17, 2024
I agree, I just don't want to lose this information b/c right now I don't have the bandwidth to figure out how to get it working on the sphinx-tag side.
Thanks for the heads up. |
jklymak commentedSep 17, 2024
Problems with examples should be issues for discussion, not tags. |
story645 commentedSep 17, 2024
Why not both? The idea here is use tags to get a holistic overview and also flag those examples in the dev version. I think that makes it easier to then write an issue, especially if the fix is more holistic. |
jklymak commentedSep 17, 2024
Because tags are for users to find things in the docs. Issues are for maintainers to track issues. Basically by tagging an example as "too much" you are issuing a judgement. To remove your judgement I would then have to open a PR to change the tag. That is backwards to how an issue works, where there are no committed changes until a PR is approved. |
timhoffm commentedSep 17, 2024
Is it sufficient to create a collection issue for "example rework"? Instead of adding a
This is much more lightweight. It can be edited without a PR and review. Also it's possible to add a few words for context to the point. The tag must stand for itself. The only downside I see is that non-core devs likely don't have the right to edit that issue. But that's not too bad. They can comment on it and we merge the comments in to one top-level list if needed. |
story645 commentedSep 17, 2024
These are tags that only show up in the dev version, which we generally don't want users using
Or request changes on the PR and have a discussion there?
That's sorta what motivated having the internal tags in the first place - to give contributors and maintainers a quick way to flag examples that need to be looked at, and using the tags machinery to get a quick list with links of those examples. The issue version ends up being a top down scan all the examples, while the tag version is "oh, looking through this example for other reasons, it also should get added to the list for review". For me a compromise would maybe be a pinned issue or template for unclear examples w/ tagging - something where the tracking issue isn't getting buried on page 3. |
jklymak commentedSep 17, 2024
I'll stand by my stance documentation tags should not be used to organize issues. There arelots of mechanisms to organize and surface issues on GitHub, specifically tags and projects, and we should use those. |
story645 commentedOct 27, 2024
removed the internal tags so this can get merged |
| # plot-type: speciality, plot-type: scatter, component: ellipse, component: patch, | ||
| # domain: statistics, |
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.
We have discussed this somewhere, but can't find the PR right now: I believe we wanted to use a standard formatting for better readability, and this can be
- one line for short lists, e.g.
.. tags:: plot-type: speciality, domain: statistics - one tag per line for longer listsSuggested change
# plot-type: speciality, plot-type: scatter, component: ellipse, component: patch, # domain: statistics, # plot-type: speciality # plot-type: scatter # component: ellipse # component: patch # domain: statistics
From a quick check, existing tags seem indeed to conform to this formatting.
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.
Yeah but this didn't need to be line wrapped/trigger the PEP 8 test and I thought that was the benchmark for "long"
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.
Yeah but this didn't need to be line wrapped/trigger the PEP 8 test and I thought that was the benchmark for "long"
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.
My definition of "long" is "does not fit on a single line". What's your cut-off? 😄
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 linter yelling
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.
But you started a second line with# domain: statistics,. To extrapolate, I could add more tags there, and when that's full, start a third line and continue.
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.
🤦♀️ sorry for being stupidly stubborn, I thought they were all on one line and didn't catch 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.
No problem 😁
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.
ok, just pushed up all the changes.
Co-authored-by: katie <katie@unrefugees.org>Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
d7ce220 intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
…569-on-v3.9.xBackport PR#27569 on branch v3.9.x (DOC: initial tags for statistics section of gallery)
…569-on-v3.9.2-docBackport PR#27569 on branch v3.9.2-doc (DOC: initial tags for statistics section of gallery)
Uh oh!
There was an error while loading.Please reload this page.
PR summary
Batch of tags for the stats functions, started by@ktnharris22 in#27235
I mostly didn't add to the tags except to flag stuff that's veering into tutorial or might belong in plot-types. I put/propose stashing internal-tags in an ifconfig b/c I think they can be useful on dev as a flag of what to work on and confusing/clutter on any of the stable versions.
PR checklist