dagster-io / dagster

An orchestration platform for the development, production, and observation of data assets.
https://dagster.io
Apache License 2.0
11.62k stars 1.46k forks source link

`@deprecated_param` confuses pylint #24009

Closed apmorton closed 1 week ago

apmorton commented 2 months ago

Dagster version

1.8.2

What's the issue?

from dagster import define_asset_job

job = define_asset_job(name="job", selection="selection", description="job")

If you run pylint on the above code it now reports:

thing.py:3:0: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return)

Removing the @deprecated_param decorator makes the message go away.

How to reproduce?

pylint 3.2.6
astroid 3.2.4
C0DK commented 2 months ago

I've gotten the same issue.

smackesey commented 1 week ago

Hi, this is a bug in pylint. I am not sure what logic pylint is using to determine what the return value of define_asset_job is but it appears to not be accounting for the @overload on deprecated_param: https://github.com/dagster-io/dagster/blob/master/python_modules/dagster/dagster/_annotations.py#L335-L342

Neither mypy, ruff, nor pyright appear to have a problem with this so I am confident saying the issue is on pylint's end. I suggest turning off that particular pylint rule either globally or for problem lines until pylint fixes the issue.

apmorton commented 1 week ago

Pylint is inference based and does not consider type hints or overloads - it parses AST and infers possible return values based on its understanding.

Pylint may be incorrectly inferring the result of this function, but this is a pretty annoying thing to have to disable on every dagster job definition.

Disabling globally isn't an option, as that would mask genuine errors.

I will try to pinpoint where the no-return inference is coming from.

jimmyxie-figma commented 1 week ago

We tried to update to the latest version of Pylint, that didn't help. Can we reopen this issue and keep track of the progress /solution here for anyone who runs into the problem while upgrading?

jimmyxie-figma commented 1 week ago

@apmorton thanks for looking into it. Keep us posted on the path to upgrade. Ty.