some small fixes and edits to work with modap by bizarreboa · Pull Request #236 · FRBs/FRB

bizarreboa · 2026-05-05T23:58:11Z

mostly added a way to change the cigale SED plot to be a png so that text can be added to it, and then several various fixes for small errors

…D and it outputs as a png instead of a pdf

…a fix with twomass querying

…ed a astropy units Mpc issue with nedlvs that I encountered

…ff into just ImportError

SunilSimha

Requesting changes.

For whatever reason, GitHub is not allowing me to create more comments. So here are the remaining ones:

Line 314 of survey_utils

I'd make the table masked here and add good masks to individual columns. Then, it becomes easy to get the combined mask by getting the "OR" of the individual column masks. Also preserves more info if the user wishes for more granularity later.

e.g.

cat = Table(cat, masked=True)
has_good = np.zeros_like(tab, dtype=bool)

for col in mag_cols:
    tab[col].mask = np.isfinite(tab[col]) & tab[col]< 90 & (tab > 0)
    has_good |= tab[col].mask

Line 273 of survey_utils

This doesn't make sense here. The combined cat for a large cone search should return several rows. Therefore, picking the best row should be a separate function implemented elsewhere. If MODaP needs this, then MODaP must implement it on whatever this function returns.

SunilSimha · 2026-05-06T14:14:23Z

        os.system('mv {:s}/{:s}_SFH.fits {:s}'.format(host.name, host.name, sfh_file))
    return
+
+def add_text_SED(host, cigale_results, out_name=None):


Seems a little hacky so are you certain there's no internal CIGALE plot argument that does this already? If not, feel free to just resolve this comment.

Checked — pcigale-plots sed has no text-annotation option; the labels are baked
into the matplotlib figure it writes, so there's nothing to hook into. Keeping the
helper.

SunilSimha · 2026-05-06T14:16:43Z

-    delta = np.trapezoid(throughput * source_flux * 
-                     10 ** (-0.4 * Alambda), x=wave) / np.trapezoid(
-                         throughput * source_flux, x=wave)
+    if getattr(np, "trapz", None) is not None:


Maybe worth adding a deprecation warning here. I believe this is for Numpy < 2.0, and so I don't see a reason to support this long term.

Alternatively, switching over to the scipy implementation if the naming there is a little more stable.

Went with np.trapezoid

SunilSimha · 2026-05-06T14:20:51Z

-                cut_photom[key] -= cut_photom['extinction_{}'.format(filt[-1])]
-                continue
+        print('Correcting filter {} for Galactic extinction'.format(filt))
+        # SDSS ### removing this because SDSS correction is sometimes different for very dusty sightlines, but TBD


I would trust SDSS extinction estimates more because I imagine they got their E(B-V) values at the location where they took their spectra as opposed to our method of using the nearest measurement within a few degrees. Worth looking into why they are different.

Agreed, the SDSS-provided value is preferred again, now behind a
_valid_extinction_value() guard so masked / NaN / -999 entries fall through to the
Gordon 2024 curve instead of silently corrupting the magnitude. Also added an optional
ext_methods dict so callers can record which correction was used per filter.

SunilSimha · 2026-05-06T14:22:41Z

 # Command line execution
 if __name__ == '__main__':
-    r_vs_dm()
+    r_vs_dm()


Can you check that there's a corresponding entry in the new pyproject.toml to make sure this script is installable when pip installing the repo?

Checked — there's no entry, and I don't think it needs one: r_vs_dm.py has no
cli()/main(), and it's one of five scripts (iteration, pz_given_dm,
pzdm_mag_crossmatch, r_vs_dm, random_assoc) run directly rather than installed.

SunilSimha · 2026-05-06T14:24:24Z

+    try:
+        in_match = np.isin(IDs, match_IDs)
+    except AttributeError:
+        # Older NumPy versions


I was under the impression that np.isin has always been available, even in v 1.26. If so, maybe we can get rid of this exception handling?

Gotcha, dropped the try/except.

SunilSimha · 2026-05-06T14:28:48Z

 photom = {}
 photom['DELVE'] = {}
 photom['DELVE']['DELVE_ID'] = 'quick_object_id'
+# photom['DELVE']['DELVE_ID'] = 'coadd_object_id'


Seems like these comments are not changing the original functionality or documenting anything. I suggest getting rid of the comment blocks in this file.

Removed all the commented-out blocks I added.

SunilSimha · 2026-05-06T14:30:42Z

        else:
            query_fields = _DEFAULT_query_fields+query_fields
-
+        import astropy.units as u


Is this import being used anywhere in the file? If not, please remove.

It is used — self.radius.to(u.deg) a few lines down — and the module has no top-level
units import, so removing it outright breaks get_catalog(). Moved it up into the
import block instead.

SunilSimha · 2026-05-06T14:37:33Z

+        mag_cols = valid_filters
+
+    # require separation column already computed in arcmin/arcsec/whatever
+    sep = cat['separation']


This is not guaranteed to be present for some surveys, I think.

I really need to make the survey catalogs and method inputs uniform. 😬

Ah, for now added an early return with a warning when separation isn't present. Also removed the call from search_all_surveys, so this function is now only reached when a caller opts in.

SunilSimha · 2026-05-06T14:41:06Z

+    if len(cat) == 0:
+        return cat
+
+    if mag_cols is None:


Pretty sure there's a catalog_utils function called get_mag_cols or some such that just gives you the columns that are present in a table. This is being used for extenction correction I believe. Prevents you from running the if statement in the loop over columns below.

catalog_utils._detect_mag_cols now used instead of iterating
valid_filters with a membership test inside the loop (dfa99dd).

… embed

…ation

bizarreboa · 2026-09-09T20:39:32Z

On pick_best_row_by_phot being called inside search_all_surveys: test_frbsurveys.test_search_all asserts
len(combined_cat) == 2, so collapsing the result to one row broke it — it just never
ran, being @remote_data. Removed the call in 8f5922f; search_all_surveys returns
every row in the cone again, and pick_best_row_by_phot stays as a public helper.

On the masked-table suggestion: adopted with one change. The provided snippet
assigns the good condition to .mask and then ORs the masks together but that would hide the usable photometry, so I inverted the polarity. The returned table is masked, so callers can see which filters actually carried the row.

Also picked up _detect_mag_cols and the missing-separation guard from the inline
comments. Branch is pushed and ready for another look.

bizarreboa and others added 21 commits October 17, 2025 14:42

added function to add text onto cigale sed plots

20f8ede

edited cigale SED plotting, now with a function to add text to the SE…

e892e99

…D and it outputs as a png instead of a pdf

small edits

7665016

some fixes

9d3e4fe

small edit to plotting backend to fix issue

9b0e1ae

more edits for modap, mainly changing sdss extinction correction and …

ed15368

…a fix with twomass querying

minor edits

b9cc862

group catalogs?

4ee4a1b

Merge branch 'main' into modap_edits

a5ccaa8

try new numpy version isin function otherwise in1d

30494c2

added new function to catch bad photometry pulls 999 values

c313eec

added lines to query more recent release of delve if desired, and fix…

bb490e4

…ed a astropy units Mpc issue with nedlvs that I encountered

added type keyerror fix

393e0d9

nump trapz

b413559

dfj

93c0a9d

Merge branch 'main' into modap_edits

16dda46

fixed thing

cc46376

fixing conflict in test_frbsurveys

542d0e5

fixed some units issues in coords

835d3b3

fixed some errors and suppressed warning messages about importing stu…

cb39292

…ff into just ImportError

Merge branch 'main' into modap_edits

ae48bfe

bizarreboa marked this pull request as ready for review May 6, 2026 00:04

bizarreboa requested review from SunilSimha and profxj May 6, 2026 00:05

bizarreboa added 4 commits May 5, 2026 20:23

Z_gas was removed but can keep

a058c1d

keep EAZYDIR warning as is

7789b91

changed some stuff back from when I was fiddling with stuff

15e7448

reorder surveys back to what it was

69fa6b6

SunilSimha requested changes May 6, 2026

View reviewed changes

Merge branch 'main' into modap_edits

fd020a6

bizarreboa added 18 commits September 9, 2026 14:55

adding luminosity_r as a valid derived

1588e28

fixed an error I get sometimes: 'float' object has no attribute 'view'

3330c08

fix pPXF noise clamp: two-stage nanmax before and after log_rebin

28ac29d

adding mosfire filters

9f86c94

add MOSFIRE bands to valid_filters

03efb6a

hoist the cigale filter name map to module level

317e133

prefer SDSS-provided extinction values; warn instead of dropping into…

21166fc

… embed

propagate line flux errors into A_V and SFR; add the [OII] SFR calibr…

4adc623

…ation

raise the pPXF NNLS iteration cap and export the fit parameters

c1db7a7

add 2MASS image aperture photometry for PSC-only sources

1cab413

widen the xmatch tolerance for 2MASS

c6ad459

catch the query errors the survey backends actually raise

c502916

use np.trapezoid instead of the deprecated np.trapz

a4aaf0b

drop the np.isin fallback in match_ids

ccdce6d

remove commented-out alternative column mappings

070dc03

move the units import to the top of the file

828e3b6

don't reduce the cone search to a single row

8f5922f

use detected mag columns and per-column masks in pick_best_row_by_phot

dfa99dd

Conversation

bizarreboa commented May 5, 2026

Uh oh!

SunilSimha left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bizarreboa Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bizarreboa commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SunilSimha left a comment •

edited

Loading

bizarreboa Sep 9, 2026 •

edited

Loading