Conversation
|
Some of this is superseded by an implementation of a standard in EXP; it makes sense to keep this but perhaps as an internal-only format. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a draft HDF5 converter for PSP format files and makes significant improvements to code documentation and style across multiple analysis modules. The changes modernize the codebase by replacing legacy file I/O with HDF5, improving docstrings to follow Google-style conventions, and refactoring calculations to use more efficient numpy methods.
Key Changes:
- Adds a new HDF5 converter class to convert PSP particle simulation files to HDF5 format
- Updates trapping analysis to use HDF5 for output instead of binary files
- Refactors kmeans implementation with improved documentation and K-means++ initialization
- Enhances pattern analysis classes with comprehensive docstrings and new BarFromCoefficients class
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 22 comments.
| File | Description |
|---|---|
| exptool/io/psp_to_hdf5.py | New HDF5 converter class with methods to convert PSP files to HDF5 format, including header and phase space data handling |
| exptool/utils/kmeans.py | Documentation improvements, code cleanup, addition of K-means++ initialization method, and refactoring for better readability |
| exptool/analysis/trapping.py | Migration from binary file output to HDF5 format, documentation improvements, error message standardization, and numpy optimization |
| exptool/analysis/pattern.py | New BarFromCoefficients class, enhanced documentation for BarTransform and BarFromFourier classes, and improved code organization |
Comments suppressed due to low confidence (2)
exptool/io/psp_to_hdf5.py:91
- Call to method HDFConverter.convert_psp_to_hdf5 with too few arguments; should be no fewer than 1.
self.convert_psp_to_hdf5()
exptool/io/psp_to_hdf5.py:148
- Except block directly handles BaseException.
except:
💡 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>
|
@michael-petersen I've opened a new pull request, #48, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@michael-petersen I've opened a new pull request, #49, to work on those changes. Once the pull request is ready, I'll request review from you. |
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: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
Co-authored-by: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
Co-authored-by: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@michael-petersen I've opened a new pull request, #50, to work on those changes. Once the pull request is ready, I'll request review from you. |
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>
|
@michael-petersen I've opened a new pull request, #51, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix unused parameters in HDFConverter.__init__
Co-authored-by: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
Specify exception types in bare except clauses
Co-authored-by: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
Co-authored-by: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
…r-one Fix HDFConverter static method inconsistency
Co-authored-by: michael-petersen <19195541+michael-petersen@users.noreply.github.com>
Fix self.oldmu initialization causing TypeError in KMeans.find_centers()
Based on a user request, I've been thinking about how to best implement HDF5 structure for file outputs. This is a draft implementation of an HDF5 phase-space system.
Still to be done: