Increase coverage for json_schema.py
#5963
Conversation
Deploying with
|
| Latest commit: |
6b64a6d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7d6f1141.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://tests-coverage-on-json-schem-onrl.pydantic-docs2.pages.dev |
| except IndexError as e: | ||
| raise ValueError('Cannot generate a JsonSchema for a zero-step ChainSchema') from e |
There was a problem hiding this comment.
It's impossible to reach this, because a chain schema will always have more than zero steps. It's being validated on the rust side.
There was a problem hiding this comment.
assert len(schema['steps']) > 0?
There was a problem hiding this comment.
Or just assert schema['steps'] it's effectively the same.
There was a problem hiding this comment.
what's the goal of adding that assertion?
There was a problem hiding this comment.
- We are eliminating a separate logic branch that is introduced by
try..except - We explicitly express our knowledge that there couldn't be an empty collection here
- We ensure that in case something changed we will get into this assert.
In this particular case it looks like [3] doesn't help much as we will get IndexError anyway.
In terms of [2], the performance degradation isn't significant but it is there.
Maybe just removing try..except and leaving a comment instead of the assert is enough here.
There was a problem hiding this comment.
I think I didn't push the code with the comment, I'm going to add it on the next PR. 🤔
|
please review |
|
Gracias 🙏 |
Selected Reviewer: @samuelcolvin