Add monitored flag#30
Conversation
…ometry. this allows for None flux sources to be included in forced phot (if minimum flux is none)
There was a problem hiding this comment.
Pull request overview
Adds a monitored boolean flag to fixed sources and solar-system objects so that sources without measured fluxes (e.g. asteroids) can be selected for forced photometry. Also reworks SourceGenerator so it tolerates None flux values, plumbs the flag through the core/client APIs and a new Alembic migration, and adds a --downsample option to the JPL parquet ingester plus a --monitored-flux-threshold-mJy option to the ACT FITS ingester.
Changes:
- New
monitoredfield onRegisteredFixedSource(Table)andSolarSystemObject(Table)with an Alembic migration;get_forced_photometry_sourcesnow filters bymonitoredand accepts an optionalminimum_flux. SourceGenerator(db variant) handles fluxless fixed sources and SSOs by branching its interpolation on a newdo_fluxflag, returningflux=Nonefromat_timewhen appropriate.- Ingest scripts updated: JPL parquet gains a downsample factor and marks all ingested objects as monitored; ACT FITS marks sources monitored if their flux is above a configurable threshold.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| socat/alembic/versions/a1b2c3d4e5f6_add_monitored_field.py | New migration adding monitored columns. |
| socat/alembic/versions/35a6a33e0a34_initial_database.py | Minor formatting (extra blank line). |
| socat/database/sources.py | Adds monitored to models and table mappings. |
| socat/database/statements.py | get_forced_photometry_sources filters on monitored; update_source supports monitored. |
| socat/core/fixed_sources.py | Threads monitored through async create_source/update_source. |
| socat/client/core.py | Abstract base updated with monitored parameter and optional minimum_flux. |
| socat/client/db.py | DB client supports monitored; SourceGenerator handles None fluxes; SolarSystemClient retains a session factory for generators. |
| socat/client/mock.py | Mock client mirrors the API changes; filters by monitored first. |
| socat/ingest/jplparquet.py | Adds --downsample, marks all ingested SSOs monitored, fixes Time handling. |
| socat/ingest/actfits.py | Adds --monitored-flux-threshold-mJy and sets monitored based on flux. |
| tests/test_db_client.py | Extends forced-photometry test with monitored/unmonitored cases. |
| tests/test_mock.py | Rewrites photometry test to cover monitored flag + missing flux. |
| tests/test_generator.py | New test_gen_no_flux covering fluxless fixed sources and SSOs. |
| tests/test_sso_mock.py | Adjusts existing SSO tests to include None-flux objects. |
| README.md | Significant rewrite of the project overview and usage docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| position: ICRS, | ||
| name: str | None = None, | ||
| flux: Quantity | None = None, | ||
| monitored: bool = False, |
There was a problem hiding this comment.
So my main thought on this is do we want a specific monitored flag, which is an attribute of the source, or do we rather want a flags attribute, which is say list[str], where the code can interact with this list and check for relevant flags like monitored? The advantage of the first approach is simplicity; the allowable flags are obvious by looking at the attributes of sources, no need for flag sanitation, etc. The advantage of the second is extensibility. If we want to add a new flag, we don't need to do a whole database migration, we don't need to add new_flag as an attribute everywhere, and we just need to write/modify the portions of the code which interact with that flag. This seems like it's a known problem/set of trade offs in DB design, maybe @JBorrow has ideas.
There was a problem hiding this comment.
yea I could go either way. forced_photometry is a core functionality of this so maybe that exists outside of 'flags' but 'flags' could contain 'pointing source' or 'extended' , etc that we want to use . and people could do custom things if they wanted.
merging updates in main which include sourcegenerator returned from solar system searches.
Add a flag for sources which are actively monitored (i.e. used in forced photometry). This allows for sources without measured fluxes to be monitored, such as asteroids.
also fix a sourcegenerator bug for ssos with None fluxes