Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-121035: Update logging flow chart to include lastResort#121036

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

Merged
vsajip merged 1 commit intopython:mainfrombessman:doc/last-resort-logging-flow
Jun 27, 2024

Conversation

bessman
Copy link
Contributor

@bessmanbessman commentedJun 26, 2024
edited
Loading

@bessmanbessman requested a review fromvsajip as acode ownerJune 26, 2024 08:57
@ghost
Copy link

ghost commentedJun 26, 2024
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

@vsajip
Copy link
Member

Thanks for this, but what process have you used? I've been thinking about replacing (or perhaps augmenting) this PNG file with something that's more easily modifiable, such as an SVG.

@bessman
Copy link
ContributorAuthor

I edited it in GIMP. Not the most convenient workflow, indeed.

If you prefer, I could look into recreating the flowchart as SVG in Inkscape or Dia. It won't look exactly the same, of course, but I'm sure I can get it reasonably close.

@vsajip
Copy link
Member

I could look into recreating the flowchart as SVG

That would be great. I think it needs to be a bit "nicer", style-wise, than the current iteration, which looks pretty old-fashioned and out of step with the overall documentation that surrounds it. If you think that's too much work, I'll understand!

@bessman
Copy link
ContributorAuthor

I think it needs to be a bit "nicer", style-wise

I'm not much of a graphic designer, so I can't really execute on this without further guidance. Do you have an example of an image in the style you would like?

For now, I have replaced the PNG with an SVG in the same style. I'll experiment a bit with font size and background to make sure it's readable.

@bessman
Copy link
ContributorAuthor

I'm working on embedding the SVG in the page HTML properly, so that the font is the same as the surrounding page text.

@vsajip
Copy link
Member

Quick work, it's coming along nicely. Things that occurred to me:

  • I would change "lastResort" to "handler of last resort"
  • The overall size of the SVG might need to be tweaked, too - at least from what I can see from the documentation preview.
  • Maybe keep your updated PNG in the repository (for now), in case we find that some browsers can't display the SVG (unlikely, I know). We can link the SVG from the documentation as you've done, but keep the PNG around in case we need to add an additional link to that.

@bessmanbessmanforce-pushed thedoc/last-resort-logging-flow branch fromefa0237 tofa836c7CompareJune 27, 2024 13:03
@bessman
Copy link
ContributorAuthor

I would change "lastResort" to "handler of last resort"

How about "lastResort handler"? My thinking is that for someone unfamiliar with thelogging module, it is immediately obvious that "lastResort" is the name of an actual Python object, and a quick ctrl+f on the same page will bring the user to an explanation of what it is. Whereas "handler of last resort" is less obvious what it is, at least to me.

vsajip reacted with thumbs up emoji

@bessman
Copy link
ContributorAuthor

bessman commentedJun 27, 2024
edited
Loading

One problem: The sphinx:class: invert-in-dark-mode command does not work with raw html. I've added a bit of CSS to the SVG which makes it aware of the browser's dark mode setting, but I haven't been able to figure out a way to make it respond to the theme selector drop down. I.e. if someone with their browser set to dark mode selects light mode from the theme selector, the diagram will look bad. Same for light mode browser + dark mode theme selector.

An alternative is to load the SVG with.. image:: instead of.. raw:: html, but then the text in the diagram won't be copyable or accessible to screen readers.

@vsajip
Copy link
Member

vsajip commentedJun 27, 2024
edited
Loading

One problem: The sphinx :class: invert-in-dark-mode

Maybe leave that for a later refinement? Accessibility is probably more important than a light/dark mismatch. To react to the theme selector would probably need a bit of javaScript.

bessman reacted with thumbs up emoji

@bessmanbessmanforce-pushed thedoc/last-resort-logging-flow branch from81841f9 tob1ce092CompareJune 27, 2024 20:10
@bessman
Copy link
ContributorAuthor

Then I think this is ready.

@vsajipvsajip added the docsDocumentation in the Doc dir labelJun 27, 2024
@vsajipvsajip merged commit237baf4 intopython:mainJun 27, 2024
28 checks passed
@vsajip
Copy link
Member

Thank you very much for this!

@vsajipvsajip added needs backport to 3.12only security fixes needs backport to 3.13bugs and security fixes labelsJun 27, 2024
@miss-islington-app
Copy link

Thanks@bessman for the PR, and@vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks@bessman for the PR, and@vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJun 27, 2024
…handler. (pythonGH-121036)(cherry picked from commit237baf4)Co-authored-by: Alexander Bessman <bessman@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJun 27, 2024
…handler. (pythonGH-121036)(cherry picked from commit237baf4)Co-authored-by: Alexander Bessman <bessman@users.noreply.github.com>
@bedevere-app
Copy link

GH-121105 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelJun 27, 2024
@bedevere-app
Copy link

GH-121106 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelJun 27, 2024
@bessmanbessman deleted the doc/last-resort-logging-flow branchJune 27, 2024 21:24
vsajip pushed a commit that referenced this pull requestJun 27, 2024
vsajip pushed a commit that referenced this pull requestJun 27, 2024
mrahtz pushed a commit to mrahtz/cpython that referenced this pull requestJun 30, 2024
@vsajip
Copy link
Member

@bessman I made some changes to the SVG to tweak elements positions and styles. I want to export the changed version to PNG, but the PNG I export from Inkscape has a transparent background. What tool did you end up using to create the SVG and PNG (including precise versions, and on which OS)? I modified the SVG by hand as I couldn't find a suitable tool that would make minimal changes to the SVG you created.

@vsajip
Copy link
Member

There's still a pending issue of syncing with the theme selector, but I think that will require changes in the pydoc theme to e.g. add a class to thebody element indicating the mode in use.

@bessman
Copy link
ContributorAuthor

What tool

I made it in Dia (v0.97+git from Ubuntu 20.04). Perhaps the .dia file should also be added to the repo?

To create the PNG, I used Dia's export feature. However, after some experimentation I've found a way to do the conversion with imagemagick (6.9.10-23 Q16 x86_64 20190101) which achieves better compression:

convert logging_flow.svg -background none -define png:compression-filter=1 -define png:compression-level=9 -define png:compression-strategy=0 logging_flow.png

@vsajip
Copy link
Member

Thanks for the info.

Perhaps the .dia file should also be added to the repo?

I've not used Dia, is the.dia file text? We don't generally add non-standard formats to the repo. If Dia can import SVG, I suppose we might not need it?

BTW I think I've now addressed the theme selector issue in#121254. Feel free to try it out. It's merged in the main branch, but not yet backported to 3.12/3.13.

@bessman
Copy link
ContributorAuthor

is the.dia file text?

It's gzip'd XML.

vsajip reacted with thumbs up emoji

@vsajip
Copy link
Member

Sadly, I've noticed that following my changes (mostly to remove inline styles from the elements in favour of CSS), Dia won't import the SVG correctly any more. See this screenshot:

dia

@bessman
Copy link
ContributorAuthor

Well, in case it's helpful, here is the .dia file:logging_flow.xml.gz

I had to change the suffix to .xml.gz for github to accept it.

@vsajip
Copy link
Member

Thanks very much.

noahbkim pushed a commit to hudson-trading/cpython that referenced this pull requestJul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this pull requestJul 17, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@vsajipvsajipvsajip approved these changes

Assignees
No one assigned
Labels
docsDocumentation in the Doc dirskip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@bessman@vsajip

[8]ページ先頭

©2009-2025 Movatter.jp