Refactor synthetic data utilities#311
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #311 +/- ##
==========================================
+ Coverage 91.68% 92.05% +0.37%
==========================================
Files 18 18
Lines 2128 2241 +113
==========================================
+ Hits 1951 2063 +112
- Misses 177 178 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
hpparvi
left a comment
hpparvi
left a comment
There was a problem hiding this comment.
Thanks @tepickering! This refactor allows for significantly cleaner code and will also be a great help with the tutorials. The only issue I could find was with trace_coeffs.
| def render(self, ctx: _RenderContext) -> np.ndarray: | ||
| trace_center = ctx.ny / 2 if self.trace_center is None else self.trace_center | ||
| trace_coeffs = ( | ||
| {"c0": 0, "c1": 50, "c2": 100} |
There was a problem hiding this comment.
The default trace_coeffs raises an error if the user sets trace_order < 2. I've opened a PR targeting this branch with a fix.
There was a problem hiding this comment.
good catch! this was pre-existing bug as well with the hard-coded defaults. i pushed a fix that should take care of it.
|
Merged! |
2 participants
This PR is basically a full re-write of
specreduce.utils.synth_data. The old architecture was too limiting and made it difficult to add new functionality. This new architecture implements aSynthImageclass that builds synthetic images by chaining method calls. This is a much cleaner, more flexible approach that is also much more easily extensible. More importantly, it makes iteration a lot more straightforward. This will be needed to appropriately model data from multi-object/fiber and echelle spectrographs.The old functions still exist, but have been refactored as thin wrappers and flagged for deprecation as of version 2.0. The docs have been updated accordingly and the examples that were in docstrings have been moved to the new dedicated
.rstfile.A feature has been added to allow a spectrum of a source to be specified. Previously sources were hard-coded to be flat spectra.