search

python-jsonschema / jsonschema

An implementation of the JSON Schema specification for Python
https://python-jsonschema.readthedocs.io
MIT License
4.52k stars 574 forks source link

fix: Ignore validators set to 'None' #1253

Closed dodslaser closed 2 months ago

dodslaser commented 2 months ago

Fixes a regression in 4.22.0 where validators set to 'None' are not ignored, causing a TypeError: 'NoneType' object is not callable on validation. In version 4.21.1 and earlier, any validator set to None would be ignored.

📚 Documentation preview 📚: https://python-jsonschema--1253.org.readthedocs.build/en/1253/

Julian commented 2 months ago

Hey there.

This isn't something I intentionally changed, but it's also not a regression really, the type of validators is (always was) documented to be callables with 4 arguments -- if you want one that does nothing just don't add it at all to extend, or add it and make it be a function which does nothing, but yeah I don't make any guarantees on what any sentinel value will do here, this all may change.

dodslaser commented 2 months ago

Hey there.

This isn't something I intentionally changed, but it's also not a regression really, the type of validators is (always was) documented to be callables with 4 arguments -- if you want one that does nothing just don't add it at all to extend, or add it and make it be a function which does nothing, but yeah I don't make any guarantees on what any sentinel value will do here, this all may change.

OK, that makes sense. I originally noticed in the source that None validators would be ignored, so I assumed it was an intended feature to ignore a keyword. When this behavior broke with 1.22.0 I wasn't sure if that was on purpose.

I guess one could argue that ignoring a validator is cleaner (and potentially very slightly faster) than repeatedly calling a no-op function, but I haven't tested that, and I doubt I would notice a significant performance hit.

Either way, for my use case I can probably replace None with a lambda *_: ... and call it a day 😄

Julian commented 2 months ago

Definitely trust the docs for what's public API yeah.

Not including the keyword at all should be even faster if you want to micro-optimize, but yeah I doubt it will show up in any case.

(Thanks regardless for the PR).