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

Make coerce working within items rule #361

Closed
wants to merge 1 commit into from

Conversation

oev81
Copy link
Contributor

@oev81 oev81 commented Feb 23, 2018

Case example:
schema = {'things': {'type': 'list', 'items': [{'coerce': int}, {'coerce': str}]}}

@oev81 oev81 force-pushed the coerce_within_items branch 2 times, most recently from 36b3ea9 to 9d06a06 Compare February 23, 2018 10:24
Copy link
Member

@funkyfuture funkyfuture left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this extension!

one thing isn't clear for me. the docs state regarding the items rule:

The items will only be evaluated if the given iterable’s size matches the definition’s.

Is this also addressed in this normalization context?

i'd like to see a test that covers the case when a value in a document throws a ValueError on coerce. the handling seems covered. yet error handling deserves some regession tests, especially the error representations.

validator = self._get_child_validator(
document_crumb=field, schema_crumb=(field, 'items'),
schema=schema)
value_type = type(mapping[field])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is type being called here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

i'd like to see a test that covers the case when a value in a document throws a ValueError on coerce.

I've added tests for ValueError and TypeError.

why is type being called here?

I used __normalize_sequence as a base. This line is added by commit eb24176 (Ensures container types are preserved in normalization). If tuple passed instead list, type will be preserved.

>>> schema = {'things': {'type': 'list', 'items': [{'coerce': int}, {'coerce': str}]}}
>>> validator = Validator(schema)
>>> validator.validate({'things': ['1', 2]})
True
>>> validator.document
{'things': [1, '2']}
>>> validator.validate({'things': ('1', 2)})
True
>>> validator.document
{'things': (1, '2')}

@funkyfuture
Copy link
Member

a'ight, cool.

I used __normalize_sequence as a base.

ah, thanks for the reminder. ;-)

just one thing, that should be a simple: when the number of fields in a sequence of a documnet is different from the amount of defined items in the relating items constraint, no normalization should be applied as this part of the document will fail later. does this make sense?

@oev81
Copy link
Contributor Author

oev81 commented Feb 24, 2018

does this make sense?

Well... It seems I understand now why you asking :)
I'm playing with cerberus less than a week. I'm learning its features while making a prototype for some real API validation. And that is how I have run into case, when coerce not working. I searched a way to convert some parameter value within a validation, and the only way I found is coerce rule. This rule is applied before validation, and now I've understood that it works only for simple cases, and doesn't work for cases with rules like oneof_schema or oneof_items, or so.

So, is there alternative to coerce? Some workaround?

Answer for your question is, yes, lengths must match :)

@funkyfuture
Copy link
Member

now, i don't get your question. due to lack of context. but i'd recommend StackOverflow for such questions.

regarding the length match; there's just a condition missing in line 688 to address this, right?

oh, and one possibility isn't covered: when an items and a schema rule are defined on a field. this may not be common but is valid. it's not hard to change the logic, but the question is in which order these would be applied. both possible orders would make sense in different contexts. :-/

@funkyfuture
Copy link
Member

@oev81 are you going to finalize this patch?

@oev81
Copy link
Contributor Author

oev81 commented Mar 12, 2018

are you going to finalize this patch?

@funkyfuture, sorry for not answering. I found the approach of coerce (conversion before validation) does not feet my task (conversion after validation, or conversion within validation).
I'm not interested in the patch now.
Please, drop the patch, if you don't need it.

@funkyfuture
Copy link
Member

@nicolaiarocci would you re-open this pr and assign me to it? i'd complete this blind spot some day.

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

Successfully merging this pull request may close these issues.

3 participants