-
Notifications
You must be signed in to change notification settings - Fork 66
Class supporting queries for GMN catalog #856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: prerelease
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,11 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import zlib | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import argparse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from RMS.Math import angularSeparationDeg | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from RMS.Logger import LoggingManager, getLogger | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Import the requests library for downloading the GMN star catalog | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -15,10 +20,139 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from urllib2 import URLError, HTTPError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import numpy as np | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from scipy.spatial import cKDTree | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from RMS.Decorators import memoizeSingle | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from RMS.Misc import RmsDateTime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime, timezone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| J2000 = datetime(2000, 1, 1, 12, 0, 0).replace(tzinfo=timezone.utc) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class Catalog: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Load a star catalogue, build a spherical KD-tree, and expose query methods. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, config, catalogue_time=None, ra_col=0, dec_col=1, mag_col=2, name_col=0, lim_mag=None): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Initialise a catalog in a spherical tree object/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Arguments: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| config[config]: RMS config instance. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Keyword Arguments: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| catalogue_time: Time point for the catalogue generation if none build for now. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ra_col: Optional, default 0 - array column with ra data in degrees. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dec_col: Optional, default 1 - array column with dec data in degrees. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mag_col: Optional, default 2 - array column of magnitude data. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name_col: Optional, default 3 - array column of star names. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.ra_col, self.dec_col, self.mag_col = ra_col, dec_col, mag_col | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.name_col = name_col | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if catalogue_time is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| catalogue_time = datetime.now(timezone.utc) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Compute the number of years from J2000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| years_from_J2000 = (catalogue_time - J2000).total_seconds() / (365.25 * 24 * 3600) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| star_catalog_status = readStarCatalog( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| config.star_catalog_path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| config.star_catalog_file, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lim_mag=np.inf, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lim_mag=np.inf, | |
| lim_mag=np.inf if lim_mag is None else lim_mag, |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an extraneous pass statement here which has no effect and looks like leftover debug code. It should be removed.
| pass |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If readStarCatalog fails, the code prints an error but continues and will raise when unpacking star_catalog_status. This should fail fast (raise an exception or return early) to avoid a confusing downstream traceback.
| pass | |
| if not star_catalog_status: | |
| print("Could not load star catalogue") | |
| if not star_catalog_status: | |
| print("Could not load star catalogue") | |
| raise RuntimeError("Could not load star catalogue from '{}' (file '{}')".format( | |
| config.star_catalog_path, config.star_catalog_file)) |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catalog.__init__ unconditionally requests additional_fields and unpacks 4 return values from readStarCatalog, but readStarCatalog returns only 3 values for non-GMN catalogs (e.g., BSC/GAIA/Sky2000). If this class is GMN-only, add a clear validation/error when the configured catalog is not GMN; otherwise handle both return shapes.
| pass | |
| if not star_catalog_status: | |
| print("Could not load star catalogue") | |
| catalog_stars, _, config.star_catalog_band_ratios, extras = star_catalog_status | |
| if not star_catalog_status: | |
| print("Could not load star catalogue") | |
| raise ValueError("Could not load star catalogue from path '{}' and file '{}'".format( | |
| config.star_catalog_path, config.star_catalog_file)) | |
| # Handle both 3- and 4-element return values from readStarCatalog. | |
| if isinstance(star_catalog_status, tuple): | |
| if len(star_catalog_status) == 4: | |
| catalog_stars, _, config.star_catalog_band_ratios, extras = star_catalog_status | |
| elif len(star_catalog_status) == 3: | |
| catalog_stars, _, config.star_catalog_band_ratios = star_catalog_status | |
| # Construct a minimal extras dict with a preferred_name field so later code works. | |
| n_stars = catalog_stars.shape[0] | |
| if (self.name_col is not None and | |
| 0 <= self.name_col < catalog_stars.shape[1]): | |
| preferred_names = catalog_stars[:, self.name_col] | |
| else: | |
| preferred_names = np.full(n_stars, "", dtype=object) | |
| extras = {"preferred_name": preferred_names} | |
| else: | |
| raise ValueError("Unexpected number of values returned by readStarCatalog: {}".format( | |
| len(star_catalog_status))) | |
| else: | |
| raise ValueError("Unexpected type returned by readStarCatalog: {}".format( | |
| type(star_catalog_status))) |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testCatQueryRaDec depends on config and log globals that are only set in the __main__ block. This makes the function fragile when imported/called from elsewhere. Consider passing config and log in as parameters (or constructing a logger/config within the test function).
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serializeQueryResults formats searched_mag inside the star_names is not None branch, but searched_mag is only assigned when star_mag is not None. If callers pass star_names without star_mag, this will raise UnboundLocalError. Either require both, or guard the magnitude formatting when star_mag is missing.
| if star_mag is not None: | |
| searched_mag = star_mag[i] | |
| if star_names is not None: | |
| searched_name = star_names[i] | |
| if last_searched_name != searched_name: | |
| output.append(f"\n\tNew search for {searched_name:20} of magnitude {float(searched_mag):4.2f}") | |
| last_searched_name = searched_name | |
| searched_mag = star_mag[i] if star_mag is not None else None | |
| searched_name = star_names[i] if star_names is not None else None | |
| if searched_name is not None and last_searched_name != searched_name: | |
| if searched_mag is not None: | |
| output.append(f"\n\tNew search for {searched_name:20} of magnitude {float(searched_mag):4.2f}") | |
| else: | |
| output.append(f"\n\tNew search for {searched_name:20}") | |
| last_searched_name = searched_name |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When no results are returned, serializeQueryResults returns a Python list, but in the non-empty case it returns a string. This inconsistent return type will break callers that always expect a string (e.g., logging). Return a string in the empty-results case as well.
| return output | |
| last_searched_name = None | |
| for i, r in enumerate(results): | |
| for name, ra, dec, mag, theta in r: | |
| if star_mag is not None: | |
| searched_mag = star_mag[i] | |
| if star_names is not None: | |
| searched_name = star_names[i] | |
| if last_searched_name != searched_name: | |
| output.append(f"\n\tNew search for {searched_name:20} of magnitude {float(searched_mag):4.2f}") | |
| last_searched_name = searched_name | |
| output.append( | |
| f"\t\tReturned name: {name:20s} RA={float(ra):8.3f} Dec={float(dec):8.3f} Mag={float(mag):5.2f} Sep={float(theta):6.3f}") | |
| else: | |
| last_searched_name = None | |
| for i, r in enumerate(results): | |
| for name, ra, dec, mag, theta in r: | |
| if star_mag is not None: | |
| searched_mag = star_mag[i] | |
| if star_names is not None: | |
| searched_name = star_names[i] | |
| if last_searched_name != searched_name: | |
| output.append(f"\n\tNew search for {searched_name:20} of magnitude {float(searched_mag):4.2f}") | |
| last_searched_name = searched_name | |
| output.append( | |
| f"\t\tReturned name: {name:20s} RA={float(ra):8.3f} Dec={float(dec):8.3f} Mag={float(mag):5.2f} Sep={float(theta):6.3f}") |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CLI wiring appears to mix up radius and lim_mag: Catalog(..., lim_mag=float(cml_args.radec[2])) uses the radius value as lim_mag, and cat.queryRaDec(ra, dec, lim_mag) passes the limiting magnitude as radius_deg. This will produce incorrect queries. Use lim_mag from cml_args.radec[3] when constructing the catalog, and pass radius to queryRaDec’s radius_deg parameter.
| cat = Catalog(config, lim_mag=float(cml_args.radec[2])) | |
| results = cat.queryRaDec(ra, dec, lim_mag) | |
| cat = Catalog(config, lim_mag=lim_mag) | |
| results = cat.queryRaDec(ra, dec, radius) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
__init__docstring and parameters disagree:name_coldefaults to 0 in the signature but the docstring says default 3, andname_colisn't used in the implementation. Either implement name column selection or remove/rename these parameters so the API matches actual behavior.