some small fixes and edits to work with modap#236
Conversation
…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
There was a problem hiding this comment.
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.
| 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): |
There was a problem hiding this comment.
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.
| 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: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Alternatively, switching over to the scipy implementation if the naming there is a little more stable.
| 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 |
There was a problem hiding this comment.
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.
| # Command line execution | ||
| if __name__ == '__main__': | ||
| r_vs_dm() No newline at end of file | ||
| r_vs_dm() |
There was a problem hiding this comment.
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?
| try: | ||
| in_match = np.isin(IDs, match_IDs) | ||
| except AttributeError: | ||
| # Older NumPy versions |
There was a problem hiding this comment.
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?
| photom = {} | ||
| photom['DELVE'] = {} | ||
| photom['DELVE']['DELVE_ID'] = 'quick_object_id' | ||
| # photom['DELVE']['DELVE_ID'] = 'coadd_object_id' |
There was a problem hiding this comment.
Seems like these comments are not changing the original functionality or documenting anything. I suggest getting rid of the comment blocks in this file.
| else: | ||
| query_fields = _DEFAULT_query_fields+query_fields | ||
|
|
||
| import astropy.units as u |
There was a problem hiding this comment.
Is this import being used anywhere in the file? If not, please remove.
| mag_cols = valid_filters | ||
|
|
||
| # require separation column already computed in arcmin/arcsec/whatever | ||
| sep = cat['separation'] |
There was a problem hiding this comment.
This is not guaranteed to be present for some surveys, I think.
I really need to make the survey catalogs and method inputs uniform. 😬
| if len(cat) == 0: | ||
| return cat | ||
|
|
||
| if mag_cols is None: |
There was a problem hiding this comment.
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.
2 participants
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