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

Silence false positive UnconsumedParameterWarning #3235

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

starhel
Copy link
Contributor

@starhel starhel commented Apr 11, 2023

UnconsumedParameterWarning is emitted even if parameters are correctly consumed.

Description

There are two cases when UnconsumedParameterWarning is false positive:

  • camelCase or parameters with dashes in name are not correctly recognized when cfg parser is used.
  • Some parameters are defined in core section of config (autoload_range, no_configure_log), but are not needed in class instance (as must be used before class is initialized).

Motivation and Context

False positive UnconsumedParameterWarning forced me to completely disable this feature as any output to stderr is treated as error in pipelines.

Have you tested this? If so, how?

I've added unitests and tested this branch in my pipeline.

@starhel starhel requested review from dlstadther and a team as code owners April 11, 2023 13:31
@starhel starhel force-pushed the fix-unconsumed-parameter-warning branch from 8b621ee to 6c5a463 Compare April 11, 2023 13:46
@starhel starhel force-pushed the fix-unconsumed-parameter-warning branch from 370789a to 59a3bcd Compare May 9, 2023 08:37
Copy link
Contributor

@andresgomezfrr andresgomezfrr left a comment

Choose a reason for hiding this comment

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

LGTM

@starhel
Copy link
Contributor Author

starhel commented Jun 7, 2023

@andresgomezfrr Do you have any idea how to fix codecov?

@andresgomezfrr
Copy link
Contributor

andresgomezfrr commented Jun 8, 2023

@andresgomezfrr Do you have any idea how to fix codecov?

@starhel Could you try to update this branch with master?

@starhel starhel force-pushed the fix-unconsumed-parameter-warning branch from 59a3bcd to e77375d Compare June 14, 2023 14:34
@starhel
Copy link
Contributor Author

starhel commented Jun 14, 2023

Branch was updated, I've forced CI to retest PR.

edit: codecov python package is no longer supported: https://github.com/codecov/codecov-python.

Do not emit UnconsumedParameterWarning when parameter name is written in camelCase or with dashes.
@starhel starhel force-pushed the fix-unconsumed-parameter-warning branch from e77375d to b877047 Compare August 28, 2023 15:51
@dlstadther dlstadther merged commit f78a067 into spotify:master Sep 7, 2023
35 checks passed
@dlstadther
Copy link
Collaborator

Apologies for not merging this sooner!

@tashrifbillah
Copy link
Contributor

tashrifbillah commented Oct 22, 2024

I am using Luigi 3.5.2, the latest. But I still have these warnings!


Edit:
I think I see the issue. The PR ignored only these parameters:

    ignore_unconsumed = {
        'autoload_range',
        'no_configure_logging',
    }

but any other parameter still raises these warnings.


I ended up solving it as:

pnlbwh/luigi-pnlpipe@4e5977a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants