Skip to content

Conversation

@cdce8p
Copy link
Member

@cdce8p cdce8p commented Feb 28, 2021

Steps

  • For new features or bug fixes, add a ChangeLog entry describing what your PR does.
  • Write a good description on what the PR does.

Description

Replaces #913
CC: @hippo91 @Pierre-Sassoulas

Type of Changes

Type
🐛 Bug fix

Related Issue

Closes #905
Closes pylint-dev/pylint#4093
Closes pylint-dev/pylint#4131
Closes pylint-dev/pylint#4145
Closes pylint-dev/pylint#3247
Closes pylint-dev/pylint#2717

@cdce8p
Copy link
Member Author

cdce8p commented Feb 28, 2021

@Pierre-Sassoulas It would be great if you could look at over one last time. Just to make sure I didn't miss something obvious. I'm running some tests at the moment to check if everything is good.

@Pierre-Sassoulas Pierre-Sassoulas mentioned this pull request Feb 28, 2021
2 tasks
@Pierre-Sassoulas
Copy link
Member

This could benefit from a rewrite using pre-commit for each commit: git rebase upstream/master -i --exec="pre-commit run --all-fles". Also it's the last thing we need to merge for 2.5.1, right ?

@Pierre-Sassoulas
Copy link
Member

Or maybe we just squash.

@cdce8p
Copy link
Member Author

cdce8p commented Feb 28, 2021

Or maybe we just squash.

I would do a squash merge

Also it's the last thing we need to merge for 2.5.1, right ?

The version needs to be updated, but otherwise I believe that's it.

--

I noticed a moment ago the the Python 3.7 test is failing. I'll have to check that first.

@cdce8p
Copy link
Member Author

cdce8p commented Feb 28, 2021

Strange, Appveyor was fine and Travis had an error for 3.7.
Anyway I pushed a small change which should trigger the CI tests, although it doesn't change much.

@cdce8p
Copy link
Member Author

cdce8p commented Feb 28, 2021

Unfortunately some more errors, this time with pylint: https://github.com/cdce8p/pylint/runs/1999076587?check_suite_focus=true#step:6:205
This seems to be more serious, so I would like to address it before merging.

and "ABCMeta" == meta.basenames[0]
and meta.locals.get("__getitem__") is not None
)
assert isinstance(meta, nodes.ClassDef)
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

I really hope all the tests pass now. Was a bit more difficult to fix than I expected.

Copy link
Member Author

@cdce8p cdce8p Feb 28, 2021

Choose a reason for hiding this comment

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

@Pierre-Sassoulas Can you reproduce the Travis issue. I unfortunately can't 😞
https://travis-ci.org/github/PyCQA/astroid/jobs/760830892

Copy link
Member

Choose a reason for hiding this comment

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

Hmm it's strange, on 2391061 I have an error but not the same than for Travis

____ ExceptionModelTest.test_oserror __

self = <tests.unittest_object_model.ExceptionModelTest testMethod=test_oserror>

    def test_oserror(self):
        ast_nodes = builder.extract_node(
            """
        try:
            raise OSError("a")
        except OSError as err:
           err.filename #@
           err.filename2 #@
           err.errno #@
        """
        )
        expected_values = ["", "", 0]
        for node, value in zip(ast_nodes, expected_values):
            inferred = next(node.infer())
>           assert isinstance(inferred, astroid.Const)
E           AssertionError: assert False
E            +  where False = isinstance(Uninferable, <class 'astroid.node_classes.Const'>)
E            +    where <class 'astroid.node_classes.Const'> = astroid.Const

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Should I just change the required version to 3.8?
Since it works with AppVeyor Link, I would assume that this is a Travis issue.

@cdce8p
Copy link
Member Author

cdce8p commented Feb 28, 2021

@Pierre-Sassoulas I think it's done. Nothing unexpected with any tests.
I'll prepare pylint-dev/pylint#4152 for the astroid update in pylint.

@Pierre-Sassoulas
Copy link
Member

Sounds good, I'll release astroid 2.5.1 asap :)

@Pierre-Sassoulas Pierre-Sassoulas merged commit 181642f into pylint-dev:master Feb 28, 2021
@cdce8p cdce8p deleted the fix-typing-alias branch February 28, 2021 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment