Validator: remove reliance on meta fields and check mandatory queries

[ml-evs]

• Closes Validator should not rely on SHOULD "meta" field "data_returned" #675 by ignoring any checks that require the use of meta->data_returned when it is not provided.
• Closes Validator should not rely on meta->data_available #677 by ignoring checks that require meta->data_available when not provided.
• Several improvements to error messages, request displays and robustness to unexpected responses
• Closes Validate mandatory query field structure_features #678 by providing fallback query values for enum fields that are normally empty.

[codecov]

Codecov Report

Merging #676 (cae2035) into master (4dced15) will decrease coverage by 0.31%.
The diff coverage is 72.22%.

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
- Coverage   93.58%   93.27%   -0.32%     
==========================================
  Files          61       61              
  Lines        3306     3344      +38     
==========================================
+ Hits         3094     3119      +25     
- Misses        212      225      +13     

Flag	Coverage Δ
project	93.27% <72.22%> (-0.32%)	⬇️
validator	66.05% <72.22%> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
optimade/validator/validator.py	82.87% <69.69%> (-1.49%)	⬇️
optimade/validator/config.py	100.00% <100.00%> (ø)
optimade/validator/utils.py	94.55% <100.00%> (+0.03%)	⬆️

---

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 4dced15...cae2035. Read the comment docs.

[CasperWA]

I have some suggested changes. It's a bit difficult for me to discern the functional changes and their consequence from the diff, but I'll give it another try once you've had a chance to respond to my suggested changes and comments.

[ml-evs]

Just fixed a couple more issues arising after testing this on some other implementations; we no longer have any internal failures on the current state of OMDB, OQMD or COD. Happy to split this PR up if that helps, but the commit history should be sensible.

[CasperWA]

Just fixed a couple more issues arising after testing this on some other implementations; we no longer have any internal failures on the current state of OMDB, OQMD or COD. Happy to split this PR up if that helps, but the commit history should be sensible.

I would argue they all should still fail due to them not supporting the mandatory query on structure_features.

• COD: https://www.crystallography.net/cod/optimade/v1/structures?filter=NOT%20structure_features%20HAS%20ALL%20%22assemblies%22
• OQMD: http://oqmd.org/optimade/v1/structures?filter=NOT%20structure_features%20HAS%20ALL%20%22assemblies%22
• OMDB: http://optimade.openmaterialsdb.se/v1.0.0/structures?filter=NOT%20structure_features%20HAS%20ALL%20%22assemblies%22

Edit: But of course, there shouldn't be internal errors in the validator. So your statement still stands (read it a bit too quickly).

[ml-evs]

I would argue they all should still fail due to them not supporting the mandatory query on structure_features.

* COD: https://www.crystallography.net/cod/optimade/v1/structures?filter=NOT%20structure_features%20HAS%20ALL%20%22assemblies%22

* OQMD: http://oqmd.org/optimade/v1/structures?filter=NOT%20structure_features%20HAS%20ALL%20%22assemblies%22

* OMDB: http://optimade.openmaterialsdb.se/v1.0.0/structures?filter=NOT%20structure_features%20HAS%20ALL%20%22assemblies%22

By internal failures I meant that they still fail validation but don't crash the validator! Though you raise a good point; we have no concept of required queries yet. This may be where we draw the line at the validator being "necessary not not sufficient" for OPTIMADE compliance. Perhaps you could raise an issue for this? I have another issue to raise at some point too, about default response fields...

[CasperWA]

By internal failures I meant that they still fail validation but don't crash the validator! (...)

Yeah, sorry. Saw that. Also edited my post.

Though you raise a good point; we have no concept of required queries yet. This may be where we draw the line at the validator being "necessary not not sufficient" for OPTIMADE compliance. Perhaps you could raise an issue for this? I have another issue to raise at some point too, about default response fields...

Done, see #678.

[CasperWA]

The None additions are not part of the "requested changes".

I think there's a discrepancy for the providers submodule here? Did you update that in this PR? I'll run the dependabot instead.

[CasperWA]

I think there's a discrepancy for the providers submodule here? Did you update that in this PR? I'll run the dependabot instead.

Just merged this in, updating the providers submodule in master.

[ml-evs]

Happy to split this into multiple PRs if this is getting unwieldy...

[CasperWA]

Happy to split this into multiple PRs if this is getting unwieldy...

With the creeping additions it quickly gets difficult to discern what is connected to what, for sure. But at the same time, I'd like to get this done.

[CasperWA]

A final comment for me, in this iteration of the PR 😅