Skip to content

Conversation

@hartwork
Copy link
Contributor

@hartworkhartwork commented Oct 8, 2025

This mimics existing method SetReparseDeferralEnabled.
.. to make it consistent with the other four Expat security methods.
@bedevere-appbedevere-appbot added docs Documentation in the Doc dir skip news labels Oct 8, 2025
@hartworkhartwork changed the title gh-90949: Recommend "hasattr" with Expat security methodsgh-90949: Recommend hasattr with Expat security methodsOct 8, 2025
…otes .. to make it consistent with Doc/library/pyexpat.rst.
@hartworkhartwork requested a review from picnixzOctober 9, 2025 13:36
@picnixz
Copy link
Member

I think I'm fine with this addition. However, because we talk about "backported" interface, we shouldn't push this until it's effectively backported (at least once). I'm actually unsure about the expanded docs. For instance:

image

I feel that the "note" boxes are way too intrusive... I think it's more important to have the note about the surprising values rather than the note on the availability (the versionadded should be enough). OTOH, if you ever add more protections, we'll have a huge block of notes, almost always the same. Here's what I can suggest: a simple .. seealso:: which refers to a section about "Availability testing" which indicates that some features are included in micro versions instead of .0 versions. That would reduce the vertical span of the page and be more readable for users in general. WDYT?

@hartwork
Copy link
ContributorAuthor

I think I'm fine with this addition. However, because we talk about "backported" interface, we shouldn't push this until it's effectively backported (at least once).

@picnixz I'm reading that as merging at least one of the unmerged backports of #139234 first — sure.

I feel that the "note" boxes are way too intrusive...

It's got a bit less loud in the meantime when adding the ! earlier — less bold and blue now:

note

I think it's more important to have the note about the surprising values rather than the note on the availability (the versionadded should be enough).

So there is a threshold where above a note gets its own box and below a box takes too much attention, makes sense.

OTOH, if you ever add more protections, we'll have a huge block of notes, almost always the same.

It would be multiple smaller blocks, not one huge block though. It's a bit less scary than that.

Btw I have no plans of adding more, new API like that will only appear if there is no way around it.

Here's what I can suggest: a simple .. seealso:: which refers to a section about "Availability testing" which indicates that some features are included in micro versions instead of .0 versions. That would reduce the vertical span of the page and be more readable for users in general. WDYT?

Personally I think that see also requires an additional click and additional energy and that alone will many users stop from ever noticing. (I also believe readability is not in danger currently.) Let me demo what we get when taking the .. note:: markers out and turning these notes back to text. Someone reading the whole method description will get the memo then, and it will not be as loud. Push coming up in a minute or two…

@hartwork
Copy link
ContributorAuthor

@picnixz pushed.

@hartwork
Copy link
ContributorAuthor

@picnixz how do you like the new version?

@picnixz
Copy link
Member

I'm sorry but I don't have a lot of time to look at the Expat related issues/PRs. At first glance it looks fine (less visual distraction).

@hugovk As someone who has better insight on accessibility & co, what would you suggest here?

@hartworkhartwork requested a review from hugovkOctober 13, 2025 23:08
@hartwork
Copy link
ContributorAuthor

@hugovk thanks for the review! How about now?

Copy link
Member

@hugovkhugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@hugovkhugovk merged commit 0c17473 into python:mainOct 14, 2025
31 checks passed
@github-project-automationgithub-project-automationbot moved this from Todo to Done in Docs PRsOct 14, 2025
@hugovk
Copy link
Member

And backport this to 3.13?

@picnixz
Copy link
Member

Actually, we didn't backported the methods to prior versions yet. I am currently starting my new work so I didn'tt have much time to focus on those PRs (currently, only main contains these new APIs)

@hugovk
Copy link
Member

OK, so should this not have been merged yet? It's only in the 3.15 docs for now, so not such a big deal if they're coming soon-ish.

@picnixz
Copy link
Member

Yes, I should have added the DO-NOT-MERGE, my fault (we discussed it in the comments but it should have been mentioned more explicitly). It's not really important as I plan to backport the PRs next week (but my work may take me some time) (half of them are ready, the other half isn't backported yet)

@hugovk
Copy link
Member

OK, let's leave this in main for now and do the backports later. Good luck with your new work!

@hartwork
Copy link
ContributorAuthor

OK, let's leave this in main for now and do the backports later.

@hugovk good plan, there is no real damage done, no worries.

Good luck with your new work!

From me as well! 🥂

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docsDocumentation in the Doc dirskip news

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@hartwork@picnixz@hugovk@StanFromIreland