Add configurable field-specific validator overrides to set filter operators as optional

[ml-evs]

Closes #1024.

[codecov]

Codecov Report

Merging #1025 (07bff38) into master (1b1225b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1025      +/-   ##
==========================================
+ Coverage   92.33%   92.34%   +0.01%     
==========================================
  Files          67       67              
  Lines        3784     3790       +6     
==========================================
+ Hits         3494     3500       +6     
  Misses        290      290              

Flag	Coverage Δ
project	92.34% <100.00%> (+0.01%)	⬆️
validator	92.34% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
optimade/validator/config.py	100.00% <100.00%> (ø)
optimade/validator/validator.py	83.11% <100.00%> (+0.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b1225b...07bff38. Read the comment docs.

[CasperWA]

Seems good to me.

Would it be possible to add a test?
Just a very simple one for checking sub-strings in a valid and invalid case, maybe?
Simply making sure the feature works.

Also - I was thinking - the models support levels are taken from what we set in the models, correct? Does this mean we need to update these as well - or is it merely because we're hitting a sub-criterium here that the support level needs to be overridden like this?

[ml-evs]

Seems good to me.

Would it be possible to add a test? Just a very simple one for checking sub-strings in a valid and invalid case, maybe? Simply making sure the feature works.

Bit tricky to test that the tests are treated as optional (we don't test this anywhere else) since it depends on the server. The validator "config" is very much not user code, and the config values are basically hardcoded to reflect the specification. We could add a validator for the config field, but I think it would be overly restrictive in the dev edge case that someone wants to patch the validator for their own system.

Also - I was thinking - the models support levels are taken from what we set in the models, correct? Does this mean we need to update these as well - or is it merely because we're hitting a sub-criterium here that the support level needs to be overridden like this?

Yes, this doesn't affect the field support, but adds handling of edge cases to the queryable nature of the field. We certainly do want to validate equality queries on chemical formulae, so queryable should definitely remain a MUST. As these are the only two cases I have found so far, I don't think it is worth restructuring our queryable field around this.

[CasperWA]

Would it be possible to add a test? Just a very simple one for checking sub-strings in a valid and invalid case, maybe? Simply making sure the feature works.

Bit tricky to test that the tests are treated as optional (we don't test this anywhere else) since it depends on the server. The validator "config" is very much not user code, and the config values are basically hardcoded to reflect the specification. We could add a validator for the config field, but I think it would be overly restrictive in the dev edge case that someone wants to patch the validator for their own system.

Fair enough. Would it make sense with a unit test at least for the functions? Making sure the if-blocks are reached as expected? Or would you rather just not test? :)

[ml-evs]

Fair enough. Would it make sense with a unit test at least for the functions? Making sure the if-blocks are reached as expected? Or would you rather just not test? :)

We could really go to town on unit tests of edge cases with the validator (mocking servers/requests, checking particular requests etc. are made) but I don't think it is worth the effort at this stage (I don't have the bandwidth, at least). I've tested this on the OQMD (who the issue affects) and a few other providers. We should see any bugs popping up in the dashboard PR runs (when we next release) which I think provides better coverage than we could in this code.