Conversation
…for DebyeGruneisen
There was a problem hiding this comment.
Pull request overview
This PR refactors the Debye-Grüneisen implementation and its tests, introducing a new plot_vt plotting API and adding a tutorial notebook demonstrating the DebyeGruneisen workflow.
Changes:
- Refactor
DebyeGruneisento use static calculation helpers and addplot_vt()for plotting vs temperature/volume. - Update
tests/test_debye.pyto align with the refactor (new plotting API + direct tests for static helpers). - Add a new example notebook for running DebyeGruneisen in Codespaces and plotting results.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
dfttk/debye.py |
Refactors processing/calculation helpers and replaces prior plotting with plot_vt() |
tests/test_debye.py |
Updates tests for the refactor and adds coverage for the static helper methods and plot_vt() |
examples/debye/Al_tutorial.ipynb |
Adds a DebyeGruneisen tutorial notebook (Codespaces setup + plotting examples) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… docstrings/comments in plot_vt, and adjust tests (DebyeGruneisen)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_debye.py
Outdated
| for type in types: | ||
| # Should raise an error if plot is called before process | ||
| with pytest.raises( | ||
| RuntimeError, match="process\\(\\) must be called before plot\\(\\)" |
There was a problem hiding this comment.
test_plot() expects the pre-process() error message to mention plot(), but DebyeGruneisen.plot_vt() raises DebyeGruneisen.process() must be called before plot_vt(). As written, this regex won’t match and the test will fail. Update the match= pattern to align with the actual plot_vt() error message (or use a looser substring/re.escape).
| RuntimeError, match="process\\(\\) must be called before plot\\(\\)" | |
| RuntimeError, match="process\\(\\) must be called before plot_vt\\(\\)" |
No description provided.