classification
Title: configparser: SectionProxy.get is silent on missing options
Type: behavior Stage: resolved
Components: Library (Lib) Versions:
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: Stig Johan Berggren, lukasz.langa, methane
Priority: normal Keywords: patch

Created on 2018-07-26 20:23 by Stig Johan Berggren, last changed 2018-08-08 15:07 by lukasz.langa. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8487 closed Stig Johan Berggren, 2018-07-26 20:27
Messages (5)
msg322448 - (view) Author: Stig Johan Berggren (Stig Johan Berggren) * Date: 2018-07-26 20:23
`get()` on a ConfigParser object behaves differently from `get()` on a section. The former raises an exception when the key does not exist and no fallback has been explicitly set. The latter returns None, with no option to raise an error for missing keys. I think this is confusing, and that both classes should have the same behaviour. I prefer raising an error, as it makes it easier to find errors such as typos in config files.

In addition, the docs state that the "parser-level `get` method
provides a custom, more complex interface, maintained for backwards
compatibility". The SectionProxy `get` method internally uses the parser-level `get`, so it seems unlikely that it is only maintained for backwards compatibility.

My proposed change is not backwards compatible, as any code that relies on a None being returned when a key does not exist would have to make this explicit through the fallback argument in `get`.

Here is the current behaviour in the latest build (3.8.0a0):

>>> import configparser
>>> c = configparser.ConfigParser()
>>> c.add_section('spam')
>>> c['spam'].get('eggs') # Returns None
>>> c['spam']['eggs']
Traceback (most recent call last):
  ...
KeyError: 'eggs'
>>> c.get('spam', 'eggs')
Traceback (most recent call last):
  ...
configparser.NoOptionError: No option 'eggs' in section: 'spam'
>>>
msg322474 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-07-27 09:11
I don't think it's worth enough to break backward compatibility.

Additionally, "section.get(key) returns None if key is not exist" is consistent with dict's behavior.

On the other hand, "parser.get(section, key)" has different signature from dict's get method.  So inconsistency is not a big problem.
msg322477 - (view) Author: Stig Johan Berggren (Stig Johan Berggren) * Date: 2018-07-27 09:27
That's fair, I didn't consider consistency with dicts' `get`. Maybe a ConfigParser object could have an option to raise errors, because they are useful for discovering errors in config files.

I still find the remark about the parser-level `get` being maintained for backwards compatibility strange (and it is eight years old), could this be improved?
msg323286 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2018-08-08 15:06
> Maybe a ConfigParser object could have an option to raise errors, because they are useful for discovering errors in config files.

For this option, use mapping access instead of `.get()`:

>>> cp['section']['key']
Traceback (most recent call last):
...
KeyError: 'key'
msg323288 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2018-08-08 15:07
> I still find the remark about the parser-level `get` being maintained for backwards compatibility strange (and it is eight years old), could this be improved?

How? Parser-level `.get()` predates mapping protocol access and has an incompatible API. Thus, we need to leave it as is, otherwise we'd break existing code.

Use mapping protocol everywhere in new code and you won't see those issues anymore.
History
Date User Action Args
2018-08-08 15:07:41lukasz.langasetstatus: open -> closed
resolution: wont fix
messages: + msg323288

stage: patch review -> resolved
2018-08-08 15:06:06lukasz.langasetmessages: + msg323286
2018-07-27 09:27:28Stig Johan Berggrensetmessages: + msg322477
2018-07-27 09:12:31methanesetnosy: + lukasz.langa
2018-07-27 09:11:49methanesetnosy: + methane
messages: + msg322474
2018-07-26 20:27:05Stig Johan Berggrensetkeywords: + patch
stage: patch review
pull_requests: + pull_request8009
2018-07-26 20:23:23Stig Johan Berggrencreate