Skip to content

gh-93607: document root attribute of iterparse #99410

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

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

Prometheus3375
Copy link
Contributor

@Prometheus3375 Prometheus3375 commented Nov 12, 2022

@Prometheus3375 Prometheus3375 force-pushed the Prometheus3375-xml-iterparse-docs branch from ff6423a to 7a568ff Compare November 22, 2022 13:51
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Copy link
Contributor

@slateny slateny left a comment

Choose a reason for hiding this comment

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

I'm no expert on this, but looks right and testing shows the same:

>>> f = io.StringIO('<data><to></to></data>')
>>> iter1 = iterparse(f)
>>> iter1.root
>>> [_ for _ in iter1]
[('end', <Element 'to' at 0x000001EF004B75B0>), ('end', <Element 'data' at 0x000001EF004B7560>)]
>>> iter1.root
<Element 'data' at 0x000001EF004B7560>

@Prometheus3375
Copy link
Contributor Author

It can be note that root attribute is always present, it is just None until the entire source is processed. In other words, iterparse_object.root is not None is a correct way to check that file is fully read while hasattr(iterparse_object, 'root') is not (if someone uses it for such checks). Thus, I am suggesting to clarify this point and also move the whole note about root a bit higher, when return object is discussed.

   Returns an :term:`iterator` providing ``(event, elem)`` pairs;
   it is also populated with a ``root`` attribute being set to ``None``.
   Once *source* is fully read, this attribute references the root element of the 
   resulting XML tree.

or

   Returns an :term:`iterator` with a ``root`` attribute providing ``(event, elem)`` pairs.
   ``root`` attribute is set to ``None``, but once *source* is fully read, 
   it references the root element of the resulting XML tree.

Both variants have weird wording in my opinion, but I couldn't think of a more acceptable one; suggestions are welcome. Maybe such clarification is not necessary at all.

@slateny
Copy link
Contributor

slateny commented Nov 30, 2022

Perhaps

   Returns an :term:`iterator` providing ``(event, elem)`` pairs;
   it has a ``root`` attribute that references the root element of the 
   resulting XML tree once *source* is fully read.

I think it may be fine to omit the mention of None and make it implicit here, but I wonder if it could also be misleading.

@Prometheus3375
Copy link
Contributor Author

I think your variant is acceptable.

Committed variant

Once source is fully read, <...> object is populated with a root attribute

states (imho) that attribute is added only at the end of parsing. This not correct.

Variant

it has a root attribute that references the root element of the resulting XML tree once source is fully read

does not hold information when attribute is added, but still notes when this attribute has a valid reference. Therefore, it is fine.

@hugovk hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 31, 2023
@hugovk hugovk enabled auto-merge (squash) October 31, 2023 16:07
@hugovk hugovk merged commit 5cc6c80 into python:main Oct 31, 2023
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 31, 2023
)

(cherry picked from commit 5cc6c80)

Co-authored-by: Prometheus3375 <35541026+Prometheus3375@users.noreply.github.com>
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 31, 2023
)

(cherry picked from commit 5cc6c80)

Co-authored-by: Prometheus3375 <35541026+Prometheus3375@users.noreply.github.com>
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 31, 2023

GH-111555 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 31, 2023
@bedevere-app
Copy link

bedevere-app bot commented Oct 31, 2023

GH-111556 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 31, 2023
hugovk added a commit that referenced this pull request Oct 31, 2023
…111556)

Co-authored-by: Prometheus3375 <35541026+Prometheus3375@users.noreply.github.com>
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
hugovk added a commit that referenced this pull request Oct 31, 2023
…111555)

Co-authored-by: Prometheus3375 <35541026+Prometheus3375@users.noreply.github.com>
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@Prometheus3375 Prometheus3375 deleted the Prometheus3375-xml-iterparse-docs branch October 31, 2023 19:08
FullteaR pushed a commit to FullteaR/cpython that referenced this pull request Nov 3, 2023
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants