Return an informative error message when spec should be a list #62
Conversation
When target is a list but spec is not you get >> ValueError: invalid literal for int() with base 10: This commit makes the error message more informative in this case.
Codecov Report
@@ Coverage Diff @@
## master #62 /- ##
==========================================
- Coverage 94.45% 94.34% -0.12%
==========================================
Files 14 14
Lines 1479 1485 6
Branches 231 232 1
==========================================
Hits 1397 1401 4
- Misses 47 48 1
- Partials 35 36 1
Continue to review full report at Codecov.
|
|
hey this looks great, thanks! |
|
Hey, thanks! I agree with Kurt, this is indeed great. I've been iterating on the idea of having a top-level exception raised to simplify the stack and make the error more actionable. I think this definitely falls into that vein. I'll need to think a bit more about this exception message-based approach. Might this make the behavior a bit version-specific? |
|
Hey - sorry for the delayed response! I see what you mean about relying on the exception message, I'm not sure how stable those messages are across python versions. What about an explicit check on whether |
glomis great, thanks for making it!This PR makes the error message more informative when your target is a list and your spec is not.
Currently when target is a list but spec is not you get
ValueError: invalid literal for int() with base 10:which doesn't immediately suggest that you forgot to make your spec a list. I've run into this a couple times now, and each time I forgot about last time and ended up having to check the source code to figure out what was going on. Next time I might finally remember, but for other people hitting this a different error message might help. With this change, the error message is
ValueError: Error indexing the target. This can happen when your target is a list and your spec is not. Did you forget to put brackets around your spec?Please let me know if this check makes more sense somewhere else, I just put it right where the error first gets raised.
Thanks!
MWE: