Skip to content
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

Add node_ancestors method #1169

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Conversation

nickdrozd
Copy link
Contributor

I've been thinking about ways to get rid of unbounded while loops in
the Pylint codebase. A common use is to loop over a node's ancestors.
The node_ancestors method centralizes this logic in one place so
that those while loops can be rewritten as for.

A few uses are made of this new method. It only comes up a few places
in Astroid, but there are many more in Pylint.

Type of Changes

Type
🔨 Refactoring

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

I think it's useful to add ancestors in the NodeNG API, I'm pretty sure I've seen it done in pylint but the logical place seems to be here.

astroid/nodes/node_ng.py Outdated Show resolved Hide resolved
@Pierre-Sassoulas Pierre-Sassoulas added the Maintenance Discussion or action around maintaining astroid or the dev workflow label Sep 13, 2021
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

This will be a great helper method! Thanks for working on it.
I only did a quick search in pylint/checkers/utils.py and already found a few places where it would be useful.

astroid/nodes/node_classes.py Outdated Show resolved Hide resolved
astroid/nodes/node_ng.py Outdated Show resolved Hide resolved
@cdce8p
Copy link
Member

cdce8p commented Sep 13, 2021

There should also be a changelog entry for it

I've been thinking about ways to get rid of unbounded `while` loops in
the Pylint codebase. A common use is to loop over a node's ancestors.
The `node_ancestors` method centralizes this logic in one place so
that those `while` loops can be rewritten as `for`.

A few uses are made of this new method. It only comes up a few places
in Astroid, but there are many more in Pylint.
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

Looks good!

As explained earlier, with the name conflict I would just leave it at node_ancestors.

--
@nickdrozd Just a small note. Could you please not force-push any new changes in future PRs? That will make reviewing them more difficult. In the end we can always squash merge them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining astroid or the dev workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants