reuse error kinds where appropriate #81
Conversation
src/errors/kinds.rs
Outdated
| @@ -118,6 +102,16 @@ pub enum ErrorKind { | |||
| LiteralSingleError, | |||
| #[strum(serialize = "literal_error", message = "Value must be one of: {expected}")] | |||
| LiteralMultipleError, | |||
| // --------------------- | |||
| // comparison errors | |||
There was a problem hiding this comment.
What I was considering, in addition to the changes present, is whether min/max length errors should get aggregated?
This would require passing the involved Py type's name or removing this information from the error message.
@samuelcolvin thoughts?
There was a problem hiding this comment.
Not quite sure what you mean?
There was a problem hiding this comment.
Apologies, I made this sentence unnecessarily unreadable.
What I meant is I wondered whether, say, DictTooShort and ListTooShort errors should become one, too, just like the comparison errors did in this very PR.
There was a problem hiding this comment.
Yes I guess so, but it'll require a context variable for "items" vs. "characters" etc.
Note, I'll also started some of these changes in #77.
There was a problem hiding this comment.
Oh, alright, I didn't realise. I guess we can close this one then :D
Codecov Report
@@ Coverage Diff @@
## main #81 +/- ##
=======================================
Coverage 93.78% 93.78%
=======================================
Files 40 40
Lines 3491 3491
Branches 26 26
=======================================
Hits 3274 3274
Misses 217 217
Continue to review full report at Codecov.
|
czotomo
changed the title
tests/validators/test_function.py
Outdated
| 'loc': [], | ||
| 'message': 'String must have at most 5 characters', | ||
| 'input_value': '12345x', | ||
| 'context': {'max_length': 5}, | ||
| 'context': {'type': 'String', 'max_length': 5, 'element_name': 'characters'}, |
There was a problem hiding this comment.
@samuelcolvin I am not sure if this 'element_name' (I fully expect you to not like that context variable and ask me to rename it :D) should be printed out. It's kinda redundant? Same goes for the type in this context, I guess. It's all explicit in the error's message anyway.
There was a problem hiding this comment.
Let's keep string (and maybe bytes) as separate enum members, but with the same output value via serialize.
Then we only need type which is fine.
There was a problem hiding this comment.
Part of the point of context is to allow translation of errors in downstream packages.
So type should be included.
|
Thanks so much. |
fixes: #76