-
Notifications
You must be signed in to change notification settings - Fork 49
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
verbs without positional arguments raise pylint(no-value-for-parameter) #465
Comments
Thanks for reporting. Let me try adding some overloads quickly to the decorator being used, to see it can silence the warnings. siuba uses generic function dispatch, which python has poor type support for, but we can at least silence warnings with very general overloads. Related to: |
I added a quick draft in this PR, but am still seeing the error, so need to dig a bit deeper..! If you have any sense for why it'd still throw pylance errors with these annotations, would love your feedback! |
@nathanjmcdougall it seems to be working now on this PR: #466 , any chance you can double check? Should be able to install the PR using... pip install git+https://github.com/machow/siuba.git@ux-type-overloads |
I've installed that PR branch but unfortunately As for pylint, looking into it, it seems that pylint has some long-standing limitations when decorators are used: I see two ways forward: make the code a little uglier for the sake of pylint users, or expect pylint users to use a standard bit of configuration when using siuba. For the first option: There may be other options. One is to not use decorators and just use Or, just suggest that siuba users add this pylint configuration which solves the issue: [tool.pylint.typecheck]
signature-mutators = ["siuba.siu.dispatchers.verb_dispatch"] If so, it may be good to add some guidance in the documentation for pylint users by compiling some standard configuration options, e.g. also the one here: |
Thanks for digging into this! This is a tough one, since it seems like pylint is doing a poor job with this specific type check, and doesn't make full use the overloads. Adding a new argument is nice as a quick fix, but will make type annotations inside concrete versions less useful, since you'd need to annotate I've been wanting to move the initial verb definitions to a module called |
When I use
mutate
, pylint gives an error:This is true for other verbs too. I have found that any time a verb is used without a positional argument it will give this error:
I suspect some wrapping needs to be done in
singledispatch2
to prevent the argspec from looking like it requires__data
as a positional argument, but that's just a hunch.I'm using
pandas==1.5.2
,siuba==0.4.2
, andpylint==2.15.8
with default configuration. For what it's worth I also getPylance
issues.The text was updated successfully, but these errors were encountered: