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

Fix TaskDoc.set_entry #853

Merged
merged 5 commits into from
Oct 12, 2023
Merged

Fix TaskDoc.set_entry #853

merged 5 commits into from
Oct 12, 2023

Conversation

janosh
Copy link
Member

@janosh janosh commented Oct 12, 2023

Fixes atomate2 defect workflow test. See materialsproject/atomate2#548 (comment) for details.

@janosh janosh added Core Any updates for Emmet-Core fix Bug fix labels Oct 12, 2023
@janosh
Copy link
Member Author

janosh commented Oct 12, 2023

Not sure why self.get_entry was required since task_id is optional in TaskDoc.get_entry:

def get_entry(
calcs_reversed: List[Calculation], task_id: Optional[str] = None
) -> ComputedEntry:

@jmmshn
Copy link
Contributor

jmmshn commented Oct 12, 2023

Also, can we add a test case to check that the entry field is populated?

PS: Notes on the behavior of TaskDoc.get_entry in a dynamic WF.

https://github.com/materialsproject/emmet/blob/6ac53ecb4b34a1e508a32cb807fa4c864c991086/emmet-core/emmet/core/tasks.py#L619C1-L626

The potcar_spec cannot be parsed since you cannot iterate through a Response object.
The oxide_type cannot be properly parsed either since that function is not wrapped in a job.

@janosh
Copy link
Member Author

janosh commented Oct 12, 2023

Also, can we add a test case to check that the entry field is populated?

Done in daa0034.

@munrojm munrojm merged commit 475c79e into main Oct 12, 2023
0 of 10 checks passed
@janosh janosh deleted the fix-task-doc-set-entry branch October 12, 2023 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Any updates for Emmet-Core fix Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants