Prepare SpectrumPY for pip packaging#156
Conversation
There was a problem hiding this comment.
💡 Codex Review
SpectrumPY-Flight/src/spectrumpy_flight/spectrum_launcher.py
Lines 590 to 593 in b2564a2
_convert_packet_file() now runs from the packaged module, but it still creates output_dir = REPO_ROOT / "HDF5". After pip installation that location is inside site-packages and cannot be created or written by normal users, so converting a packet downloaded via the launcher will fail even before subprocess.run is invoked. Use the new cache directory (e.g. ~/.spectrumpy_flight/HDF5) or another user‑writable location instead of the package directory.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def run_impactbook() -> Path: | ||
| from ImpactBook import ImpactBook | ||
| from .ImpactBook import ImpactBook | ||
|
|
||
| ImpactBook(channel_names, trcdir=str(directory), ExperimentName=experiment_name) | ||
| return (REPO_ROOT / "HDF5" / f"{experiment_name}.h5").resolve() |
There was a problem hiding this comment.
Avoid writing ImpactBook output into package directory
When running the GUI from an installed wheel, REPO_ROOT points to the package inside site-packages, which is typically read‑only for normal users. run_impactbook() writes results to REPO_ROOT / "HDF5" / <experiment>.h5, so a pip‑installed copy will raise PermissionError before any processing completes. The output directory should be moved to a user‑writable cache (as done for API downloads) rather than the package installation folder.
Useful? React with 👍 / 👎.
| package_root = resources.files("spectrumpy_flight") | ||
|
|
||
| for default in ("HDF5", "Data"): | ||
| resource = package_root.joinpath(default) | ||
| if resource.exists(): |
There was a problem hiding this comment.
Replace unsupported importlib.resources exists check
The new _collect_olivine_inputs() uses resources.files("spectrumpy_flight").joinpath(default) and then calls resource.exists(). Traversable objects returned by importlib.resources do not implement exists, so the test will raise an AttributeError before any assertions run. Use resource.is_dir()/is_file() or as_file instead of exists to keep the test runnable.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_6902449889348320ad0b5875b57eb4e1