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

Allow specification of provider field descriptions/units etc. in config file #1096

Merged
merged 2 commits into from Mar 5, 2022

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Mar 4, 2022

Closes #1095.

This PR also enables some old tests for provider fields that we had disabled.

@codecov
Copy link

codecov bot commented Mar 4, 2022

Codecov Report

Merging #1096 (86c9ba4) into master (b5cd435) will increase coverage by 0.09%.
The diff coverage is 100.00%.

❗ Current head 86c9ba4 differs from pull request most recent head 242b22a. Consider uploading reports for the commit 242b22a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1096      +/-   ##
==========================================
+ Coverage   92.53%   92.63%   +0.09%     
==========================================
  Files          67       67              
  Lines        3806     3813       +7     
==========================================
+ Hits         3522     3532      +10     
+ Misses        284      281       -3     
Flag Coverage Δ
project 92.63% <100.00%> (+0.09%) ⬆️
validator 92.63% <100.00%> (+0.09%) ⬆️

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

Impacted Files Coverage Δ
optimade/server/mappers/entries.py 97.11% <ø> (ø)
optimade/server/config.py 91.30% <100.00%> (ø)
...made/server/entry_collections/entry_collections.py 97.63% <100.00%> (ø)
optimade/server/routers/info.py 95.45% <100.00%> (ø)
optimade/server/schemas.py 100.00% <100.00%> (ø)
optimade/validator/validator.py 83.66% <0.00%> (+0.55%) ⬆️

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 b5cd435...242b22a. Read the comment docs.

@JPBergsma
Copy link
Contributor

This seems a useful addition as I am still struggling with getting the provider specific fields of the Trajectories endpoint.
I'll try to have a look at it this weekend.

@ml-evs
Copy link
Member Author

ml-evs commented Mar 4, 2022

This seems a useful addition as I am still struggling with getting the provider specific fields of the Trajectories endpoint. I'll try to have a look at it this weekend.

Cheers, I'm just having some issues getting the validator action working, it seems like the installing of the package is missing some reqs (pyyaml) when installed for the action for some reason... (solved)

@ml-evs ml-evs force-pushed the ml-evs/beefy_provider_fields branch from ef4c899 to 242b22a Compare March 4, 2022 18:18
Copy link
Contributor

@JPBergsma JPBergsma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still wondered whether it would be worthwhile to make a function/method for returning the "name" field.
Now you have to do extra type checks at three places in the code.
Other than that, it looks good to me.

@ml-evs
Copy link
Member Author

ml-evs commented Mar 5, 2022

I still wondered whether it would be worthwhile to make a function/method for returning the "name" field. Now you have to do extra type checks at three places in the code. Other than that, it looks good to me.

Thanks for the review @JPBergsma! Point taken, I tried to avoid over-engineering this but the balance might not be quite right --- I think it's fine for now as there shouldn't be any other places where this check is necessary, if that changes then we can refactor.

@ml-evs ml-evs merged commit d768275 into master Mar 5, 2022
@ml-evs ml-evs deleted the ml-evs/beefy_provider_fields branch March 5, 2022 20:22
@ml-evs ml-evs added enhancement New feature or request server Issues pertaining to the example server implementation config For issues/PRs related to the server config. ergonomics Features that improve the usability of the package labels Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config For issues/PRs related to the server config. enhancement New feature or request ergonomics Features that improve the usability of the package server Issues pertaining to the example server implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow provider field descriptions to be provided in the config
2 participants