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

Spurious unsubscriptable-object error #3328

Closed
fluffy-critter opened this issue Jan 4, 2020 · 3 comments
Closed

Spurious unsubscriptable-object error #3328

fluffy-critter opened this issue Jan 4, 2020 · 3 comments
Labels
Bug 🪲 Control flow Requires control flow understanding

Comments

@fluffy-critter
Copy link

Possibly related to #2416

Steps to reproduce

I believe this is the minimal amount of code necessary to reproduce the bug.

""" Functions for manipulating images stored within local content """

class LocalImage:
    """ The basic Image class, which knows about the base version and how to
    generate renditions from it """

    def _get_rendition(self, **kwargs):
        """ implements get_rendition and returns tuple of out_rel_path,size,pending """
        _, box = self.get_rendition_size(kwargs)

        if box:
            box = (box[0], box[1], box[2], box[3])


    def get_rendition_size(self, spec):
        """
        Wrapper to determine the overall rendition size and cropping box

        Returns tuple of (size,box)
        """

        width = spec.get('width', 100)
        height = spec.get('height', 100)

        mode = spec.get('resize', 'fit')

        if mode == 'fit':
            return self.get_rendition_fit_size(width, height)

        if mode == 'fill':
            return self.get_rendition_fill_size(width, height)

        if mode == 'stretch':
            return self.get_rendition_stretch_size(width, height)

        raise ValueError("Unknown resize mode {}".format(mode))

    @staticmethod
    def get_rendition_fit_size(input_w, input_h):
        """ Determine the scale-crop size given the provided spec """
        return (input_w, input_h), None

    @staticmethod
    def get_rendition_fill_size(input_w, input_h):
        """ Determine the scale-crop size given the provided spec """
        return (input_w, input_h), (0, 0, input_w, input_h)

    @staticmethod
    def get_rendition_stretch_size(input_w, input_h):
        """ Determine the scale-crop size given the provided spec """
        return (input_w, input_h), None

Current behavior

When I run pylint on this, I get errors:

************* Module pylint_repro
pylint_repro.py:14:19: E1136: Value 'box' is unsubscriptable (unsubscriptable-object)
pylint_repro.py:14:37: E1136: Value 'box' is unsubscriptable (unsubscriptable-object)
pylint_repro.py:15:19: E1136: Value 'box' is unsubscriptable (unsubscriptable-object)
pylint_repro.py:15:37: E1136: Value 'box' is unsubscriptable (unsubscriptable-object)

If I split lines 21 and 22 off into a separate function:

    @staticmethod
    def _thunk_adjust_box(crop, box):
        if crop and box:
            box = (box[0] + crop[0], box[1] + crop[1],
                   box[2] + crop[0], box[3] + crop[1])
        elif crop:
            box = (crop[0], crop[1], crop[0] + crop[2], crop[1] + crop[3])

        return box

and replace line 21 with box = self._thunk_adjust_box(crop, box), the error disappears.

If in the method get_rendition_size I remove the lines for if mode == 'fit' the error disappears. However, the error remains if I leave that one present and remove the mode == 'stretch' lines.

Curiously, if I move the mode == 'fit' branch to go after the mode == 'fill' branch, the error also disappears.

This is peculiar since the value of box could be either tuple or NoneType regardless of the order of those branches, and anyway where the linter is throwing the error, box is guaranteed to not be NoneType.

This also still occurs if I change the box check to explicitly check for tuple, absolutely guaranteeing that box is a tuple in that code path:

        if crop and isinstance(box, tuple):
            box = (box[0] + crop[0], box[1] + crop[1],
                   box[2] + crop[0], box[3] + crop[1])

Adding appropriate type hinting has no effect.

Changing the class functions to be module-level functions causes the error to go away, which seems potentially interesting.

Expected behavior

This error should not be emitted, as box is always guaranteed to be a tuple on that code path, and the order of the proxy method branches within get_rendition_size shouldn't matter for that determination anyway.

pylint --version output

pylint 2.4.4
astroid 2.3.3
Python 3.7.4 (default, Sep  7 2019, 18:27:02) 
[Clang 10.0.1 (clang-1001.0.46.4)]
@PCManticore
Copy link
Contributor

Thanks for the great report. This issue is caused by the fact that pylint does not understand isinstance or if blocks, meaning that if a variable might be None in a branch and the code checks against that, pylint will ignore the if clause entirely, leading to false positives such as these ones. Almost all the topic-control-flow labeled issues are caused by this lack of isinstance / if guard understanding.

@PCManticore PCManticore added Bug 🪲 Control flow Requires control flow understanding labels Jan 5, 2020
@mbyrnepr2
Copy link
Member

I can't reproduce on pylint version 2.13; perhaps this issue can be closed.

@DanielNoord
Copy link
Collaborator

Thanks @mbyrnepr2!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Control flow Requires control flow understanding
Projects
None yet
Development

No branches or pull requests

4 participants