Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomplete output of optimade-validator #568

Closed
merkys opened this issue Oct 27, 2020 · 4 comments · Fixed by #569
Closed

Incomplete output of optimade-validator #568

merkys opened this issue Oct 27, 2020 · 4 comments · Fixed by #569
Assignees

Comments

@merkys
Copy link
Member

merkys commented Oct 27, 2020

I have tried optimade-validator from 6f34399:

$ ~/.local/bin/optimade-validator https://www.crystallography.net/cod-test/optimade/structures -t structures
/home/andrius/.local/lib/python3.8/site-packages/optimade/server/config.py:167: UserWarning: Unable to find config file in default location /home/andrius/.optimade.json, using the built-in default settings instead.
  warnings.warn(
.✖

Last line comes without a newline. I really have no .optimade.json, but I would expect the validator continue with built-in settings.

@ml-evs ml-evs self-assigned this Oct 27, 2020
@ml-evs
Copy link
Member

ml-evs commented Oct 27, 2020

Hi @merkys, I don't think this is a regression (I get the same result before that PR was merged). If you turn up the verbosity -vv then you get the full breakdown of that single test failure. The config warning does seem spurious here, and should be fixed.

The checks run by the -t flag are quite simple: get the URL provided and try to deserialize it with our EntryResponseMany model. The response field changes from 6f34399 only get used when running the entire test suite, as they require scraping of supported fields in e.g. /info/structures/. Hopefully you'll find the full validator tests more useful for you now as they now allow None values for SHOULDs inside the validator, the next change is to allow them inside the models too (#560).

We could definitely consider making -t a bit smarter in this regard, or at least document it more clearly.

@merkys
Copy link
Member Author

merkys commented Oct 28, 2020

Oops, I indeed forgot the -vv, sorry for the noise!

Nevertheless, what seems strange is warnings.warn( without a closing parenthesis, and .✖ without a following newline symbol. In combination these two appear as incomplete output, as one would expect a closing parenthesis and a newline.

@ml-evs
Copy link
Member

ml-evs commented Oct 28, 2020

Nevertheless, what seems strange is warnings.warn( without a closing parenthesis, and .✖ without a following newline symbol. In combination these two appear as incomplete output, as one would expect a closing parenthesis and a newline.

Python normally prints the source code of the warning invocation when showing user warnings like this, I think perhaps the .✖ has flushed stdout one line too many. The PR I made will remove this warning anyway, so no drama!

@merkys
Copy link
Member Author

merkys commented Oct 28, 2020

Python normally prints the source code of the warning invocation when showing user warnings like this, I think perhaps the .✖ has flushed stdout one line too many. The PR I made will remove this warning anyway, so no drama!

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants