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

Optional default can’t be empty list #63

Closed
klmr opened this issue Jun 25, 2019 · 3 comments
Closed

Optional default can’t be empty list #63

klmr opened this issue Jun 25, 2019 · 3 comments

Comments

@klmr
Copy link

klmr commented Jun 25, 2019

I‘m unsure whether this is a bug report or a feature request (i.e. whether the current behaviour is by design).

My desired behaviour is this: I would like to specify an optional map element that can be either a non-empty sequence or a non-empty comma-separated list. If the element isn’t specified, it should default to an empty list.

To clarify, here are valid examples:

a:
  - foo
b: bar
b: bar

And this is an invalid example:

a:
b: bar

Currently, the following works (from strictyaml import *):

schema = Map({Optional('a', None): Seq(Str()), 'b': Str()})
list(map(lambda t: load(t, schema), ['a:\n  - x\nb:\n', 'b:\n']))

However, when changing the default to [], it fails:

schema = Map({Optional('a', []): Seq(Str()), 'b': Str()})

InvalidOptionalDefault: Optional default for 'a' failed validation:
Expected a non-empty list, found an empty list.
Use EmptyList validator to serialize empty lists.

Likewise, the following fails, with a different error, at a different time:

schema = Map({Optional('a', []): CommaSeparated(Str()), 'b': Str()})
list(map(lambda t: load(t, schema), ['a: x\nb:\n', 'b:\n']))

IndexError: string index out of range (in strictyaml/utils.py L96)

As suggested in the first error message above the following works, but it describes a different schema (namely, it allows an existing, empty a)!

schema = Map({Optional('a', list()): EmptyList() | Seq(Str()), 'b': Str()})
@crdoconnor
Copy link
Owner

IndexError: string index out of range (in strictyaml/utils.py L96)

This exception should definitely never be happening, thanks for reporting it. I've got a fix ready to roll out in the next release. That one should work seamlessly now.

As suggested in the first error message above the following works, but it describes a different schema (namely, it allows an existing, empty a)!

Can you explain what the issue you have with this is? As far as I can see it just allows two different ways of representing the same valid data - one implicit (optional; no key) and one explicit (key, with blank string). This was by design: optional default is supposed to be a default - not a value that isn't representable if the key is present.

I use something like this (EmptyList() | Seq()) quite a lot, actually - and if, like me, you want "empty list" to be a valid value for a... allowing an empty "a" makes sense, no?

I'm open to the idea of disallowing the blank string representation, but I'd like to hear a coherent justification why - i.e. realistic examples/use cases.

I'm also looking at a related issue #62 - so any input on that would be welcome, too. Two people having a similar kind of issue makes me wonder if there's something I'm missing.

@klmr
Copy link
Author

klmr commented Jun 26, 2019

optional default is supposed to be a default - not a value that isn't representable if the key is present.

That makes sense. Since EmptyList() | … is what I’m currently using as a “workaround” I guess there’s no harm sticking with it. 👍

@klmr klmr closed this as completed Jun 26, 2019
@klmr
Copy link
Author

klmr commented Jun 26, 2019

By the way, I did see #62 but that’s unrelated.

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