Conversation
…ated by model_type
There was a problem hiding this comment.
Pull request overview
Adds a new polynomial-based pointing residual model to Mapcat and updates persistence/tests to support storing/retrieving this new model type alongside the existing constant model.
Changes:
- Introduces
PolynomialPointingModelwith coefficient fitting, prediction, and basic statistics helpers. - Updates
PointingResidualTable.residual_modelto store either a constant or polynomial model via JSON (TypeAdapter-backed deserialization). - Expands pointing tests to cover constant-model behavior and basic polynomial fitting/prediction.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_pointing.py |
Updates unit imports and adds tests for constant and polynomial pointing models. |
mapcat/pointing/poly.py |
New polynomial pointing model implementation (fit + predict + stats). |
mapcat/pointing/const.py |
Adjusts ConstantPointingModel.predict to subtract offsets. |
mapcat/database/pointing_residual.py |
Allows storing either constant or polynomial pointing model in residual_model. |
mapcat/database/json.py |
Switches JSON deserialization to Pydantic TypeAdapter (enables unions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JBorrow
left a comment
There was a problem hiding this comment.
Some minor comments. It seems kinda overcomplicated for a polynomial model but that's ok. Is there a reason we can't just use https://numpy.org/doc/stable/reference/generated/numpy.polyfit.html
…ghts since they were redundant
|
@JBorrow other changes have been included; removed separate ra,dec uncertianty + weights; just use a weights input since they're degenerate and the user can decide what weights to use. |
add the polynomial pointing model to mapcat