Skip to content

configparser.SectionProxy.get* methods aren't correctly typed #1543

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

Closed
OddBloke opened this issue Aug 12, 2017 · 5 comments
Closed

configparser.SectionProxy.get* methods aren't correctly typed #1543

OddBloke opened this issue Aug 12, 2017 · 5 comments

Comments

@OddBloke
Copy link
Contributor

On instantiation, configparser.SectionProxy takes as an argument the parser that it's a section of and uses setattr to populate itself with partially applied converters (i.e. methods that start with get) that the parser it is passed has.

Currently, this is stubbed as:

class SectionProxy(MutableMapping[str, str]):
    def __init__(self, parser: RawConfigParser, name: str) -> None: ...

    <snip>

    # SectionProxy can have arbitrary attributes when custon converters are used
    def __getattr__(self, key: str) -> Callable[..., Any]: ...

which means that ConfigParser()['section'].getboolean('key') type checks as Any, whereas it should in fact be bool.

@OddBloke
Copy link
Contributor Author

OddBloke commented Aug 12, 2017

I think we can cover the majority of use cases by simply copying the definitions down from RawConfigParser and removing the first argument (section, which is partially applied at runtime).

This won't help people who are subclassing RawConfigParser to add converters (as in https://github.com/OddBloke/jenkins-job-linter/blob/master/jenkins_job_linter/config.py#L26-L39). Those converters via SectionProxy will always type-check as returning Any; this isn't any worse than today.

@ambv
Copy link
Contributor

ambv commented Aug 21, 2017

Go for it.

@OddBloke
Copy link
Contributor Author

Cool, looking now. The test case for this issue (thanks to @reddraggone9 in #1527 (comment)):

from configparser import ConfigParser

config = ConfigParser()
config.read_dict({'section': {'key': 'false'}})
assert config['section'].getboolean('key') is False

reveal_type(config['section'].getboolean('key'))
reveal_type(config['section'].getfloat('key'))
reveal_type(config['section'].getint('key'))

That currently outputs:

1543.py:7: error: Revealed type is 'Any'
1543.py:8: error: Revealed type is 'Any'
1543.py:9: error: Revealed type is 'Any'

but should output

1543.py:7: error: Revealed type is 'builtins.bool'
1543.py:8: error: Revealed type is 'builtins.float'
1543.py:9: error: Revealed type is 'builtins.int'

OddBloke pushed a commit to OddBloke/typeshed that referenced this issue Aug 21, 2017
These methods are partially applied (with the section that is being
proxied in SectionProxy) at runtime, so will always be present.

Fixes python#1543.
@OddBloke
Copy link
Contributor Author

The commit above fixes this, but I committed it on top of #1527 so I'll wait for that to be merged to rebase and propose an MP.

@ambv
Copy link
Contributor

ambv commented Aug 21, 2017

#1527 is merged.

OddBloke pushed a commit to OddBloke/typeshed that referenced this issue Aug 21, 2017
These methods are partially applied (with the section that is being
proxied in SectionProxy) at runtime, so will always be present.

Fixes python#1543.
ambv pushed a commit that referenced this issue Aug 21, 2017
These methods are partially applied (with the section that is being
proxied in SectionProxy) at runtime, so will always be present.

Fixes #1543.
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

No branches or pull requests

2 participants