Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 am curious, why previously both
iteratorandIterParseIteratornames were deleted, but now onlyIterParseIterator? And what is the purpose of this statement in the first place? I was thinking thatiterator.__closure__stores references to these objects; therefore, unnecessary references should be deleted. However, as per my checks, closure stores only referenced variables inside;pullparser,close_sourceandwrin this case.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 also noticed that
it.root = Nonewas deleted. This fact is not documented, but this may still cause unintended errors on the user side if they useroot.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 think you are right about the
it.root = None. I did not intend a behavioral change here, so it seems like a good idea to add it back.I don't think the
delstatements matter one way or the other. They look like they break a cycle, but not really, but they also are harmless.