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

configparser.ConfigParser.values() should return a ValuesView[configparser.SectionProxy] #11547

Open
fizbin opened this issue Mar 8, 2024 · 2 comments
Labels
stubs: false positive Type checkers report false errors

Comments

@fizbin
Copy link

fizbin commented Mar 8, 2024

Consider these two similar python programs, which have identical behavior:

#! /usr/bin/env/python

import configparser
import re
import sys

if __name__ == "__main__":
    for filename in sys.argv[1:]:
        cparser = configparser.ConfigParser(interpolation=None)
        cparser.read(filename)
        changed = False
        for _ignored, section in cparser.items():
            if section.name == "DEFAULT":
                continue
            for k, v in section.items():
                if re.fullmatch(r"\d+", v):
                    section[k] = str(int(v) + 1)
                    changed = True
        if changed:
            with open(filename, "wt", encoding="utf-8") as fp:
                cparser.write(fp)

and

#! /usr/bin/env/python

import configparser
import re
import sys

if __name__ == "__main__":
    for filename in sys.argv[1:]:
        cparser = configparser.ConfigParser(interpolation=None)
        cparser.read(filename)
        changed = False
        for section in cparser.values():
            if section.name == "DEFAULT":
                continue
            for k, v in section.items():
                if re.fullmatch(r"\d+", v):
                    section[k] = str(int(v) + 1)
                    changed = True
        if changed:
            with open(filename, "wt", encoding="utf-8") as fp:
                cparser.write(fp)

(The only difference between the two programs is invoking cparser.items() or cparser.values())

The first program type-checks in mypy without any issues, but the second program:

/tmp/incints.py:13: error: "Mapping[str, str]" has no attribute "name"  [attr-defined]
/tmp/incints.py:17: error: Unsupported target for indexed assignment ("Mapping[str, str]")  [index]
Found 2 errors in 1 file (checked 1 source file)

The problem is that ConfigParser objects accept assignment of arbitrary str -> str mappings but when you pull a value back out of a ConfigParser, the value pulled out is always a configparser.SectionProxy. (see the __getitem__ and __setitem__ methods on configparser.RawConfigParser)

Note that this affects not only the return type of values but also the return type of __getitem__; it should be perfectly well-typed to write cparser[some_str_value].name.

@srittau srittau added the stubs: false positive Type checkers report false errors label Mar 11, 2024
@srittau
Copy link
Collaborator

srittau commented Mar 11, 2024

I don't know the exact underlying problem, but in the stub we mimic the implementation by overriding items() and inheriting values() from MutableMapping. Potentially, we should derive from MutableMapping[str, SectionProxy], instead of MutableMapping[str, _Section], but I'm not sure whether that's the right solution or not.

@fizbin
Copy link
Author

fizbin commented Mar 14, 2024

Potentially, we should derive from MutableMapping[str, SectionProxy], instead of MutableMapping[str, _Section], but I'm not sure whether that's the right solution or not.

It isn't. The issue is that for purposes of setting values, any Mapping[str, str] will work, but when you get a value, what you get back is always a SectionProxy.

See the implementation of these two methods on RawConfigParser, from which ConfigParser descends:

    def __getitem__(self, key):
        if key != self.default_section and not self.has_section(key):
            raise KeyError(key)
        return self._proxies[key]

    def __setitem__(self, key, value):
        # To conform with the mapping protocol, overwrites existing values in
        # the section.
        if key in self and self[key] is value:
            return
        # XXX this is not atomic if read_dict fails at any point. Then again,
        # no update method in configparser is atomic in this implementation.
        if key == self.default_section:
            self._defaults.clear()
        elif key in self._sections:
            self._sections[key].clear()
        self.read_dict({key: value})

I suspect that the solution is to, in the stub, override items, values and __getitem__ to return types based on SectionProxy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Development

No branches or pull requests

2 participants