astroid icon indicating copy to clipboard operation
astroid copied to clipboard

Problem with partial and pickle

Open renefritze opened this issue 3 years ago • 5 comments

I'm not using astroid directly, but discovered this issue downstream using sphinx-autoapi. It seems I've run into an edge case with partial and the pickle module. I've added a new test case to astroid to showcase my problem: https://github.com/PyCQA/astroid/compare/master...renefritze:conditional_test_pickle

With the last commit on that branch the test suite succeeds: https://travis-ci.com/github/renefritze/astroid/jobs/487475875 Dropping the last commit, the suite fails: https://travis-ci.com/github/renefritze/astroid/jobs/487452325 The difference being https://github.com/PyCQA/astroid/commit/307f3acbfaa8bac3e7e4f1f0d0f38a965e54c53d

renefritze avatar Mar 29 '21 08:03 renefritze

@renefritze thanks for your report. Would you mind opening a PR on astroid please?

hippo91 avatar Apr 09 '21 12:04 hippo91

Sure thing.

renefritze avatar Apr 09 '21 12:04 renefritze

Reopening because I did not understand that the merge request was only illustrative

Pierre-Sassoulas avatar Aug 05 '21 09:08 Pierre-Sassoulas

Is there anything more I can do to help that doesn't require in-depth astroid knowledge?

renefritze avatar Nov 25 '21 09:11 renefritze

I have investigated this a little.

 def test_conditional_definition_partial_pickle(self) -> None:
        """Regression test for a conditional defintion of a partial function wrapping pickle.

        Reported in https://github.com/PyCQA/astroid/issues/925
        """
        module = resources.build_file(
            "data/conditional_pickle_import/conditional_importer.py"
        )

        dump_nodes = module.lookup("dump")[1]
        assert len(dump_nodes) == 2

        # Check the function defintion node
        func_def_nodes = list(dump_nodes[0].infer())
        assert len(func_def_nodes) == 1
        assert isinstance(func_def_nodes[0], FunctionDef)

        # Check the partial node
        partial_node = list(dump_nodes[1].infer())
        assert len(partial_node) == 1
        assert isinstance(partial_node[0], objects.PartialFunction)

This should pass with:

# conditional_importer
import pickle

if False:

    def dump(obj, file, protocol=None):
        pass


else:
    from functools import partial

    dump = partial(pickle.dump, protocol=0)

The problem is that pickle.dump can be either pickle.dump or _pickle.dump. https://github.com/PyCQA/astroid/blob/0d1211558670cfefd95b39984b8d5f7f34837f32/astroid/brain/brain_functools.py#L77

On that line in the functools brain we take the first value, which is the _pickle one (I believe). We don't infer its arguments and therefore we fail here: https://github.com/PyCQA/astroid/blob/0d1211558670cfefd95b39984b8d5f7f34837f32/astroid/brain/brain_functools.py#L98-L99

We will need a tip for a pickle brain I think to solve this. I don't have much experience with adding brains so I probably won't get to this very soon but for anybody looking to fix this: I believe this is where we should be looking.

DanielNoord avatar Dec 30 '21 19:12 DanielNoord