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

Update client.py #418

Merged
merged 3 commits into from
Oct 27, 2021
Merged

Update client.py #418

merged 3 commits into from
Oct 27, 2021

Conversation

jmmshn
Copy link
Collaborator

@jmmshn jmmshn commented Oct 27, 2021

For materials with no valid charge density task_id like mp-1104953 the current version gets an empty list at line 756 then errors out at the sorting step.
This change should fix that. Also, I think it makes sense to return None here since we are not guaranteeing every single charge density.

For materials with no valid charge density task_id like `mp-1104953` the current version gets an empty list at line 756 then errors out at the sorting step.
This change should fix that.  Also, I think it makes sense to return `None` here since we are not guaranteeing every single charge density.
@jmmshn jmmshn requested a review from mkhorton October 27, 2021 04:39
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2021

Codecov Report

Merging #418 (bb24955) into main (5bcdc9f) will decrease coverage by 0.04%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #418      +/-   ##
==========================================
- Coverage   92.61%   92.56%   -0.05%     
==========================================
  Files          63       63              
  Lines        2031     2032       +1     
==========================================
  Hits         1881     1881              
- Misses        150      151       +1     
Impacted Files Coverage Δ
src/mp_api/client.py 96.00% <66.66%> (-0.65%) ⬇️

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 5bcdc9f...bb24955. Read the comment docs.

@@ -755,11 +761,16 @@ def get_charge_density_from_material_id(self, material_id: str) -> Optional[Chgc
)
results = self.charge_density.search(task_ids=task_ids)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This can return empty sometimes

black

black
@munrojm
Copy link
Member

munrojm commented Oct 27, 2021

Thanks @jmmshn !

@munrojm munrojm added the release:patch Patch release label Oct 27, 2021
@munrojm munrojm merged commit 8c4670c into main Oct 27, 2021
@mkhorton
Copy link
Member

Thanks! Sorry missed the review request until now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:patch Patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants