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

False positive with E1136 unsubscriptable-object for use in or after is None guard #1498

Open
sam-s opened this issue May 24, 2017 · 49 comments
Labels
Control flow Requires control flow understanding Enhancement ✨ Improvement to a component Help wanted 🙏 Outside help would be appreciated, good for new contributors High priority Issue with more than 10 reactions Needs astroid constraint Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@sam-s
Copy link

sam-s commented May 24, 2017

Steps to reproduce

  1. Create file z.py
a = None
while True:
    if a is None or a["1"] == 0:
        a = {"1": 1}
    else:
        break
print("Done")
  1. run pylint z.py

Current behavior

E:  3,20: Value 'a' is unsubscriptable (unsubscriptable-object)

Expected behavior

no errors, no warnings

pylint --version output

pylint 1.7.1, 
astroid 1.5.2
Python 2.7.13 (default, Dec 18 2016, 07:03:39) 
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]
@rogalski rogalski added the Control flow Requires control flow understanding label May 31, 2017
@erjiang
Copy link

erjiang commented Jul 7, 2017

This has hit us too. We are using a variable that is initialized to None, but then gets assigned to dict in a loop.

I think something like this will trigger it too:

points = [{"lat": 39, "lon": -92}, {"lat": 39.1, "lon": -92.1}]

furthest_west = None
for p in points:
    if furthest_west is None:
        furthest_west = p
    else:
        if p['lon'] < furthest_west['lon']:
            furthest_west = p

not sure how you would solve this, seems like the inferencer would have to go into the if/else clauses to see what might get assigned to the variable to figure out that it's Union(None, dict).

@hughes
Copy link

hughes commented Mar 14, 2018

Even checking isinstance(thing, list) immediately prior to using thing will trigger this.

rjarry added a commit to rjarry/buildbot that referenced this issue Mar 15, 2018
This check is flaky and sometimes reports false-positive errors.

  pylint-dev/pylint#1498

Disable the check since we cannot rely on it to find report actual bugs.

Signed-off-by: Robin Jarry <robin@jarry.cc>
@PCManticore
Copy link
Contributor

Yes @hughes that's a problem that's still crippling pylint even today (but hopefully with the new number of committers we have, we might tackle it at some point). I'm referring here to the fact that pylint does not quite grasp control flow, that is, if you use isinstance, pylint will happily infer all potential values that a variable could have, disregarding the branch hints.

@PCManticore PCManticore added the Enhancement ✨ Improvement to a component label Mar 16, 2018
@ch3ck
Copy link

ch3ck commented Sep 11, 2018

I had this problem today opening a config file.

@kelvinau
Copy link

+1

@sdmunozsierra
Copy link

sdmunozsierra commented Oct 12, 2018

I have this false positive too. I reference a variable instanced as none in the main object class, and then change it in function from an inherited class.
Code example below; incomplete for brevity.

class Response():
    def __init__(self):
        self.raw_response = None

    def filter_raw_response(self):
        if not self.raw_response:
            return None
        num_datapoints = len(self.raw_response['Datapoints'])   # Error shows here
        if num_datapoints == 0:
            return None

class SpecificResponse(Response):

    def craft_response(self)
        # Variable instanced here:
        self.raw_response = self.client.get_metric_statistics()

@jiarongqiu
Copy link

+1

I got this too when I tried to iterate over candidates, a matrix like variable. It reports item as NoneType and raise E1136 error. But, actually, it is a list.

candidates = [item for item in val_dataset if item[-1] == i]
candidates = random.sample(candidates,100)
for item in candidates:
    img_path = os.path.join(data_dir,item[0])
    out_path = os.path.join(output_folder,str(i))
    os.system('cp %s %s'%(img_path,out_path))

@lucmos
Copy link

lucmos commented Nov 15, 2018

False positive even if the custom class implements the __getitem__ method.

@melyux
Copy link

melyux commented Dec 3, 2018

Will this ever be fixed then?

@PCManticore
Copy link
Contributor

@melikyuksel Unless someone proposes a patch, it's unlikely.

@nilselde
Copy link

+1

I got this too when I tried to iterate over candidates, a matrix like variable. It reports item as NoneType and raise E1136 error. But, actually, it is a list.

candidates = [item for item in val_dataset if item[-1] == i]
candidates = random.sample(candidates,100)
for item in candidates:
    img_path = os.path.join(data_dir,item[0])
    out_path = os.path.join(output_folder,str(i))
    os.system('cp %s %s'%(img_path,out_path))

I had the same issue when using random.sample

@sgornick
Copy link

Would this actually be a false positive? Seems to me it is following the intended behavior.

Used when there is a reference to an object that initialized as a non-subscriptable object

http://pylint-messages.wikidot.com/messages:e1136

@ravinderkhatri
Copy link

ravinderkhatri commented Jul 15, 2019

@sgornic
The solution there is suggestion to used [] to initialize an empty list instead of using None to remove the error but doing that cause an unwanted/undesirable behavior in python.
See here
https://nikos7am.com/posts/mutable-default-arguments/
+1 I am experiencing the same issue while initializing a variable with None and rewriting this with some other values given that program runs without an error

digitalfrost added a commit to digitalfrost/Uranium that referenced this issue Aug 19, 2019
The warning is because args in initialized as None
See pylint-dev/pylint#1498
False positive with unsubscriptable-object...
@andy-maier
Copy link

There is also issue #2063 that seems to describe the same error.

@andy-maier
Copy link

I also get this with the built-in Exception.args and believe it is the same error:

class HTTPError(Exception):

    def __init__(self, status, reason):
        super(HTTPError, self).__init__(status, reason)

    @property
    def status(self):
        return self.args[0]  # pylint reports: E1136: Value 'self.args' is unsubscriptable

    @property
    def reason(self):
        return self.args[1]  # same

@alexyarochkin
Copy link

Just make your list Python3 compatible:
i.e.:
my_list = [1, 2, 3, 4]
for _my in list(my_list):
pass

@jakeleventhal
Copy link

jakeleventhal commented Jun 26, 2020

I'm also seeing this with __getitem__ implemented

@olbapjose
Copy link

I'm getting the same with a Pandas DataFrame:

endog = dta["colName"]  # [E1136 unsubscriptable-object] Value 'dta' is unsubscriptable
...
dta[["c1", "c2"]]     # [E1136 unsubscriptable-object] Value 'dta' is unsubscriptable

Output of python --version: Python 3.6.10 :: Anaconda, Inc.

@technillogue
Copy link

a similar thing also occurs with bidict

@belm0
Copy link
Contributor

belm0 commented Nov 11, 2021

I see.

It would be a small quality of life improvement, but even without using the type annotation, pylint could certainly infer that foo can't be None (the initialized value), acknowledge that it doesn't know the type, and refrain from omitting the error.

@DanielNoord
Copy link
Collaborator

Perhaps I don't understand your examples but in both examples the unsubscriptable-object seems correct? In the first bool(0) will be False and thus func returns None whereas in the second example foo is initialised as None and isn't reassigned before the following line.

@belm0
Copy link
Contributor

belm0 commented Nov 11, 2021

the expression foo and foo[0] implies that foo can't possibly be indexed if it's None

but to clarify the example:

foo: Optional[Tuple] = None
# ... code that assigns a tuple to foo
print(foo and foo[0])  # unsubscriptable-object  (??)

it's actually a very common case, since variables declared type Optional[some_indexible_type] and initialized to None are very common

@DanielNoord
Copy link
Collaborator

Ah my bad, that's indeed quite obvious and a pattern we use ourselves. Should have examined the example better.

However, it is not quite that easy to add such control flow recognition to pylint. In fact, right now (I think) we do nothing of that type of control flow. Personally I have started to add some flow recognition for if ... elif, which is easier since we can check the parent node and check whether the current node is the child of an If.
The control flow needed for this example is something like checking whether we are in nodes.Comparison, whether the object being subscripted is in any of the preceding nodes of the Comparison, whether any of those nodes imply that the object is subscriptable, for example, if object is an int it would not be subscriptable but isinstance(foo, Tuple) would. Without any existing systems adding such control flow recognition is quite a daunting task and probably one of the reasons why this issue is still open and why we have so many open control flow issues.

@belm0
Copy link
Contributor

belm0 commented Nov 11, 2021

Another short-term idea: without fully supporting type hints, if pylint could at least see that the var is declared as a union including at least one indexible type, it should not emit unsubscriptable-object due to the high chance of false positive.

@dpinol

This comment was marked as spam.

@pstahlhofen
Copy link

+1
In my case, the object that is said to be unsubscriptable even though it actually is subscriptable is returned by a function from a library, so I cannot do any type-hinting or such to get rid of the wrong error message.

@matkoniecz
Copy link

following code also triggers it

maybe disable it by default as very likely to produce false positives?

    all_entries = []
    for key in ["surface", "surface:note"]:
        for entry in dubious_surfaces_requiring_notes.surface_values_that_trigger_note_creation(key):
            value = entry[0]
            comment = entry[1]
            all_entries.append((key, value, comment))
    for data in random.sample(all_entries, 6):
        print(data)
        key = data[0]
        value = data[1]
        comment = data[2]
        scan_for_tag(key, value, comment)

@optimaltayfun
Copy link

optimaltayfun commented Jan 12, 2024

I am getting this false positive as well. Below is the code triggering this:

from sqlalchemy.orm import Mapped, mapped_column
class UserDB(Base):
    """
    User database model.
    """
    __tablename__ = "users"
    user_id: Mapped[int] = mapped_column(Integer, primary_key=True, index=True, autoincrement=True, nullable=False)

The documentation from sqlalchemy describes how to use Mapped in the following link:
https://docs.sqlalchemy.org/en/20/orm/mapping_styles.html

But this is causing false positives with pylint:
app/user_auth/database.py:49:13: E1136: Value 'Mapped' is unsubscriptable (unsubscriptable-object)

My pylint version:

pylint 3.0.3
astroid 3.0.2
Python 3.12.1 (main, Dec  8 2023, 05:40:51) [GCC 11.4.0]

@nkalpakis21
Copy link

i see this has been open for awhile. is there trouble getting a dev to fix this issue? what is blocking this for so long

@krispharper
Copy link

Like @optimaltayfun and several others, I'm getting this while upgrading to SQLAlchemy 2.0. Lots of other people will hit this doing the same thing, I think.

@matkoniecz
Copy link

matkoniecz commented Oct 23, 2024

@nkalpakis21

what is blocking this for so long

because noone implemented fix for this

you can submit pull request fixing this, hire someone to do this or hope that someone else will do either

as someone who is developer on open source projects: this kind of comment is neither helpful nor encouraging, and if someone is doing it as unpaid hobby (or poorly paid one) it will sooner or later result in them dropping such project altogether

note that ability to skip unhelpful/broken rules is present in pylint (and I use it to suppress many rules I consider unhelpful), you can use it here

(and this issue has both Help wanted "🙏Outside help would be appreciated, good for new contributors" and Needs PR "This issue is accepted, sufficiently specified and now needs an implementation" labels)

@nkalpakis21
Copy link

@nkalpakis21

what is blocking this for so long

because noone implemented fix for this

you can submit pull request fixing this, hire someone to do this or hope that someone else will do either

as someone who is developer on open source projects: this kind of comment is neither helpful nor encouraging, and if someone is doing it as unpaid hobby (or poorly paid one) it will sooner or later result in them dropping such project altogether

note that ability to skip unhelpful/broken rules is present in pylint (and I use it to suppress many rules I consider unhelpful), you can use it here

(and this issue has both Help wanted "🙏Outside help would be appreciated, good for new contributors" and Needs PR "This issue is accepted, sufficiently specified and now needs an implementation" labels)

sorry, I was genuinely trying to understand the bottleneck to the solution

@Pierre-Sassoulas
Copy link
Member

Control flow is hard, it requires generic and intimidating changes in astroid (the underlying code representation of pylint) and it's inference engine (the infer function on each node of the astroid's abstract syntax tree) that is going to affect a lot of code, thus a variety of very strange and unexpected code too. If the code is too clever or too dynamic it's harder (I don't know if that's the case here). The volunteers maintainers of this project are already busy trying to follow the upstream rhythm of changes from python core to support the latest interpreter, and keeping up with the reviews of all proposed MR takes time too. (Tidelift pays me to fix security issues and prevent supply chain attack, which I do, but everything else is us giving away free time). Offering a bounty somewhere (issue hunt, github sponsor, etc.) or implementing the change yourself would probably speed things up.

@nhairs
Copy link

nhairs commented Oct 24, 2024

Hi @Pierre-Sassoulas,

Would this fall into your $1000 one-time medium bug category?

For everyone else:

I'd be happy to help sponsor getting this fixed but I can't really afford to do it myself. Maybe we can band together 10-20 people who can all donate a bit to help get us there (if you'd be willing to donate $50-100 maybe react with a 🚀 to this comment).

@Pierre-Sassoulas
Copy link
Member

This issue would requires a big new feature in astroid (make astroid understand control flow better) and would fix numerous nearly duplicate issues, so it's probably more akin to a large project. To be honest at this point I have too much on my plate to handle this even for a large bounty, but maybe @jacobtylerwalls or @DanielNoord could be interested (or someone on IssueHunt, from a country with low cost of living for whom the monetary compensation would be worth months of work).

@jacobtylerwalls
Copy link
Member

#1498 (comment) is a separate issue from the original post. I'll open a new issue for it.

@jacobtylerwalls jacobtylerwalls changed the title False positive with E1136 unsubscriptable-object False positive with E1136 unsubscriptable-object for use in or after is None guard Oct 24, 2024
@jacobtylerwalls
Copy link
Member

Actually, that SQLAlchemy issue looks like #9549, which has a workaround at #9549 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Control flow Requires control flow understanding Enhancement ✨ Improvement to a component Help wanted 🙏 Outside help would be appreciated, good for new contributors High priority Issue with more than 10 reactions Needs astroid constraint Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests