Skip to content

Phonopy fix density of states plot#582

Merged
jan-janssen merged 1 commit intomainfrom
phonopy_plot
Nov 4, 2025
Merged

Phonopy fix density of states plot#582
jan-janssen merged 1 commit intomainfrom
phonopy_plot

Conversation

@jan-janssen
Copy link
Copy Markdown
Member

@jan-janssen jan-janssen commented Nov 4, 2025

Summary by CodeRabbit

  • Refactor
    • Optimized internal data handling in phonon density of states visualization to reduce unnecessary data structure overhead. No changes to user-facing functionality or behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

The plot_dos function in the phonons helper module was refactored to remove single-element tuple wrapping from dos_energies and dos_total variable assignments, passing arrays directly to axis.plot for simplified handling.

Changes

Cohort / File(s) Summary
Tuple Unwrapping in DOS Plotting
atomistics/workflows/phonons/helper.py
Removed single-element tuple indirection from dos_energies and dos_total assignments in plot_dos function, passing arrays directly to plotting methods instead

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Simple structural refactor with no behavioral changes
  • Single function modification in a localized scope
  • Verify that array-direct passing maintains plot output consistency

Poem

🐰 A tuple unwrapped, arrays now free,
DOS plots simpler, clean as can be!
No indirection, just data so bright,
Phonons still phononing—everything right! 🎵

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Phonopy fix density of states plot' directly describes the main change - fixing a density of states plotting function in the phonon workflow helper module.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch phonopy_plot

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d15e17c and 8ad8c0c.

📒 Files selected for processing (1)
  • atomistics/workflows/phonons/helper.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: unittest_old
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.14)
  • GitHub Check: unittest_qe
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.11)
  • GitHub Check: unittest_siesta
  • GitHub Check: unittest_matrix (windows-latest, 3.14)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.12)
  • GitHub Check: unittest_matrix (macos-latest, 3.14)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.13)
  • GitHub Check: unittest_mace
  • GitHub Check: unittest_sphinxdft
  • GitHub Check: unittest_gpaw
  • GitHub Check: coverage
  • GitHub Check: notebooks
  • GitHub Check: unittest_grace
🔇 Additional comments (1)
atomistics/workflows/phonons/helper.py (1)

468-469: LGTM! Correct fix for matplotlib plotting.

Removing the single-element tuple wrapping is the right approach. Matplotlib's axis.plot() expects arrays directly, not tuples containing arrays, so this change ensures the DOS data is plotted correctly.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.46%. Comparing base (5684ac5) to head (8ad8c0c).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
atomistics/workflows/phonons/helper.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #582   +/-   ##
=======================================
  Coverage   85.46%   85.46%           
=======================================
  Files          43       43           
  Lines        2545     2545           
=======================================
  Hits         2175     2175           
  Misses        370      370           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jan-janssen jan-janssen merged commit 72bec60 into main Nov 4, 2025
28 of 29 checks passed
@jan-janssen jan-janssen deleted the phonopy_plot branch November 4, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant