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

Issues accessing PurePath.parents by index in Python 3.10 #5832

Closed
merelcht opened this issue Feb 23, 2022 · 14 comments · Fixed by #7105
Closed

Issues accessing PurePath.parents by index in Python 3.10 #5832

merelcht opened this issue Feb 23, 2022 · 14 comments · Fixed by #7105
Labels
Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code python 3.10
Milestone

Comments

@merelcht
Copy link

merelcht commented Feb 23, 2022

Bug description

We're running pylint as part of our CircleCI builds, which we run on Python 3.7, 3.8, 3.9 and have now added 3.10.

We're using pylint version 2.12.2, and the checks succeed on all versions < python 3.10, but on 3.10 we see the following errors:

E1101: Instance of 'tuple' has no 'resolve' member (no-member)
for:

starter_path = Path(__file__).parents[3].resolve()

I0021: Useless suppression of 'abstract-class-instantiated' (useless-suppression)
for:

from filelock import FileLock

root_tmp_dir = tmp_path_factory.getbasetemp().parent
lock = root_tmp_dir / "semaphore.lock"
    with FileLock(lock):  # pylint: disable=abstract-class-instantiated
          spark = _setup_spark_session()

If the above failures are fixed for Python 3.10 the checks will then fail on all builds with version < Python 3.10.

Command used

pylint -j 4 --disable=missing-docstring,redefined-outer-name,no-self-use,invalid-name,protected-access,too-many-arguments,too-many-public-methods,use-implicit-booleaness-not-comparison tests

Pylint output

This is the output for the 3.10 build:

************* Module tests.framework.cli.conftest
tests/framework/cli/conftest.py:117:19: E1101: Instance of 'tuple' has no 'resolve' member (no-member)
************* Module tests.extras.datasets.spark.conftest
tests/extras/datasets/spark/conftest.py:38:0: I0021: Useless suppression of 'abstract-class-instantiated' (useless-suppression)

Expected behavior

I would expect pylint to give the same output for the various Python versions.

Pylint version

The versions used for the respective Python builds:

pylint 2.12.2
astroid 2.9.3
Python 3.7.11 (default, Jul 27 2021, 14:32:16) [GCC 7.5.0]

pylint 2.12.2
astroid 2.9.3
Python 3.8.12 (default, Oct 12 2021, 13:49:34) [GCC 7.5.0]

pylint 2.12.2
astroid 2.9.3
Python 3.9.7 (default, Sep 16 2021, 13:09:58) [GCC 7.5.0]

pylint 2.12.2
astroid 2.9.3
Python 3.10.0 (default, Dec 21 2021, 13:36:04) [GCC 7.5.0]
@merelcht merelcht added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Feb 23, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Question and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 23, 2022
@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Feb 23, 2022

Thank you for opening the issue.

Some checks have voluntarily different output for different python version. For example typing with dict or list needs to import from __future__ in earlier python interpreter. In 2.12, we introduced a new option py-version that permits to analyze code for a python version that may differ from your current python interpreter. Could you try to set the py-version ?

It's possible that some checkers still do not take it into account properly (the tuple error seems fishy here).

@merelcht
Copy link
Author

Hi @Pierre-Sassoulas, thanks for your quick response. Do you have any docs on how to use the py-version option? Is that just in the cli command?

@Pierre-Sassoulas
Copy link
Member

This is a global option, (pylint --py-version=3.8)

If you think we can make it clearer there's an issue to make the doc better: #5038, do not hesitate to add a comment there :)

@merelcht
Copy link
Author

Thanks, the doc explanation makes sense 🙂

Unfortunately, it doesn't make a difference for my checks..

@Pierre-Sassoulas Pierre-Sassoulas added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning python 3.10 and removed Question labels Feb 23, 2022
@deepyaman
Copy link

We're running pylint as part of our CircleCI builds, which we run on Python 3.7, 3.8, 3.9 and have now added 3.10.

We're using pylint version 2.12.2, and the checks succeed on all versions < python 3.10, but on 3.10 we see the following errors:

E1101: Instance of 'tuple' has no 'resolve' member (no-member) for:

starter_path = Path(__file__).parents[3].resolve()

@MerelTheisenQB With regards to this example, the reason for different behavior in Python 3.10 is actually because .parents changed in 3.10 to accept a slice. I don't know how pylint works, but I assume it doesn't figure out that passing a non-slice index to .parents will necessarily yield a Path object, and never a tuple. @Pierre-Sassoulas Do you know whether the pylint parser should be able to distinguish something like this?

With regards to the solution, the simple thing to do is change it to:

starter_path = Path(__file__).resolve().parents[3]

as even the pathlib docs recommend calling resolve() before arbitrarily walking paths upwards.

Haven't looked into the second issue at all.

@Pierre-Sassoulas
Copy link
Member

Thank you for the investigation @deepyaman, this indeed look like something that require an astroid update.

@deepyaman
Copy link

@Pierre-Sassoulas Any chance you can provide a bit of guidance as to what that update might look like? I've been keen to understand how Astroid works, and this looks like a simple case to start with.

My guess is that it would mean adding a brain like:

if PY310_PLUS:
    def _path_transform():
        return astroid.parse('''
        class Path:
            @property
            def parents(self):
                return (Path(), Path())
        ''')

    register_module_extender(astroid.MANAGER, 'pathlib', _path_transform)

This way, I assume pylint/whatever is using Astroid would infer a tuple of Paths for Path.parents[1:3] or a Path for Path.parents[3]. A couple things I couldn't understand from the docs/looking through other brains:

  1. If I register a module extender, do I need to provide the definition for everything in Path, or I can do it like above just for Path.parents?
  2. Let's say we call Path.parents[1:3] or Path.parents[3] now; would it cause an issue that the stub of the parents method is just returning a tuple of length 2, or does Astroid/pylint just look at the types and see what a slice/int argument would return?

Happy to make a PR if preferable, but thought I'd ask in case I'm way off on this implementation.

@deepyaman
Copy link

deepyaman commented Mar 3, 2022

Just to provide an update, I've gotten to the following astroid/brain/brain_pathlib.py:

# Copyright (c) 2022 Deepyaman Datta <deepyaman.datta@utexas.edu>

# Licensed under the LGPL: https://www.gnu.org/licenses/old-licenses/lgpl-2.1.en.html
# For details: https://github.com/PyCQA/astroid/blob/main/LICENSE

from astroid import inference_tip
from astroid.const import PY310_PLUS
from astroid.exceptions import UseInferenceDefault
from astroid.manager import AstroidManager
from astroid.nodes.node_classes import Attribute, Slice, Subscript


def _looks_like_parents_subscript(node):
    return (
        isinstance(node, Subscript)
        and isinstance(node.value, Attribute)
        and node.value.attrname == "parents"
    )


def infer_parents_subscript(subscript_node, context=None):
    if isinstance(subscript_node.slice, Slice):
        raise UseInferenceDefault

    yield from subscript_node.value.expr.infer(context=context)


if PY310_PLUS:
    AstroidManager().register_transform(
        Subscript, inference_tip(infer_parents_subscript), _looks_like_parents_subscript
    )

Currently trying to figure out why UseInferenceDefault doesn't give back a tuple, since I thought the whole issue was that it was returning a tuple by default... I was being dumb and checked if isinstance(subscript_node, Slice) instead of if isinstance(subscript_node.slice, Slice). 🤦

@Pierre-Sassoulas
Copy link
Member

Sorry for the late reply :) I'm not an expert in astroid to be honest but this looks very nice, thank you for opening an astroid PR !

@jacobtylerwalls jacobtylerwalls added this to the 2.15.0 milestone Jun 29, 2022
@jacobtylerwalls jacobtylerwalls removed the Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning label Jun 29, 2022
@jacobtylerwalls jacobtylerwalls changed the title Different pylint outcome for various Python versions Issues accessing PurePath.parents by index in Python 3.10 Jun 29, 2022
@jacobtylerwalls jacobtylerwalls added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Jun 29, 2022
jacobtylerwalls added a commit to jacobtylerwalls/pylint that referenced this issue Jun 30, 2022
jacobtylerwalls added a commit to jacobtylerwalls/pylint that referenced this issue Jun 30, 2022
@Cielquan
Copy link

Cielquan commented Jul 8, 2022

I guess this issue will be solved with the 2.12 release of astroid then?

@DanielNoord
Copy link
Collaborator

Yes and after pylint actually supports 2.12. There are some changes we'll need to make, but I think we're nearing the release of 2.12 which is definetly the first step!

@galah92
Copy link

galah92 commented Aug 9, 2022

I'm getting the same issue with the following code:

cwd = Path(__file__).resolve().parents[1]
apps_paths = cwd.glob('apps/*')

On Python 10 as well. Is there an expected fix? Using pylint==2.14.5.

@Cielquan
Copy link

Cielquan commented Aug 9, 2022

In 2.14.5 astroid 2.12 is not supported. Therefore the issue is not fixed in this release.

https://github.com/PyCQA/pylint/blob/v2.14.5/setup.cfg#L50

EDIT: But with PR #7153 astroid 2.12 support was merged into main. So the next release could include this fix.

@Pierre-Sassoulas
Copy link
Member

You can follow the release of pylint 2.15 by checking the 2.15.0 milestone. We still have some blocker and most maintainers are vacationning right now :) it's possible to install the current main using pip with git+SSH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code python 3.10
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants