Skip to content

test_graph: add filename only to pdf list #3972

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 5, 2024
Merged

test_graph: add filename only to pdf list #3972

merged 1 commit into from
Aug 5, 2024

Conversation

drew-parsons
Copy link
Contributor

The pdfs list must be constructed from filenames only without other path components, i.e. use last entry from split()

Summary

Major changes:

  • fix: fix list of pdf files in test_draw (test_graph)

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • [x ] Tests added for new features/fixes (test already present).

Closes: #3969

@drew-parsons
test_graph: add filename only to pdf list
5f0726b 
The pdfs list should be constructed from filenames only without other path components, i.e. use last entry from split()
@drew-parsons drew-parsons mentioned this pull request Aug 5, 2024
Closed
@DanielYang59
Copy link
Contributor

DanielYang59 commented Aug 5, 2024 •
edited
Loading

Thanks for the fix. I'm still looking into the issue, as I just installed pygraphviz in CI and I was having a different error for that particular test for some reason (both in CI and locally) https://github.com/materialsproject/pymatgen/actions/runs/10246364887/job/28343300325?pr=3786.

Update: Issued fixed in c67abbc and your reported issue recreated successfully. Also I believe your proposed fix is correct. Thanks!

@DanielYang59 DanielYang59 mentioned this pull request Aug 5, 2024
Merged
@janosh janosh added tests Issues with or changes to the pymatgen test suite fix Bug fix PRs data viz PRs and issues about pymatgen plotting functionality labels Aug 5, 2024
janosh
janosh approved these changes Aug 5, 2024
View reviewed changes
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

thanks @drew-parsons!

@janosh janosh merged commit 4ffec03 into materialsproject:master Aug 5, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janosh janosh janosh approved these changes

@shyuep shyuep Awaiting requested review from shyuep shyuep is a code owner

@mkhorton mkhorton Awaiting requested review from mkhorton mkhorton is a code owner

Assignees
No one assigned
Labels
data viz PRs and issues about pymatgen plotting functionality fix Bug fix PRs tests Issues with or changes to the pymatgen test suite
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TestStructureGraph.test_draw TypeError: unhashable type: 'list'
3 participants
@drew-parsons @DanielYang59 @janosh