search

astropy / photutils

Astropy package for source detection and photometry. Maintainer: @larrybradley
https://photutils.readthedocs.io
BSD 3-Clause "New" or "Revised" License
246 stars 137 forks source link

[WIP] Add ofilter? #375

Closed pllim closed 3 years ago

pllim commented 8 years ago

Don't know if this will ever happen, but if it is supposed to happen, I am opening an issue here so it is not forgotten. As part of JWST DADF photometry sprint, I took up the task to convert IRAF ofilter algorithm for sky fitting to Python. Here is a gist of the results.

Should we add ofiltsky.py to photutils? If so, where?

c/c @hcferguson, @eteq, @larrybradley

larrybradley commented 8 years ago

It would need to be rewritten as a background class (see for example MeanBackground in #370) to use the current object-oriented interface. But it needs to be based off #370, so you should wait until that is merged.

pllim commented 8 years ago

Whether it should really be in the background class or not also depends on #374.

larrybradley commented 8 years ago

@pllim I haven't looked at your function yet, but to be used in centroiding and background estimation, it should be split into two parts. The first part should be a basic centroiding function, i.e. take in a 1D array and use the algorithm to give me the centroid. For example, the functions (e.g. DAOPHOT) that use this to calculate 2D centroids calculate them from the 1D marginal x and y distributions.

The second part should be a Background class (using the framework in #370). Here the class would compute the data histogram (and whatever else is needed) and then you can call the ofilter centroid function (defined above) to find the background value from the histogram.

pllim commented 8 years ago

So, the centroid portion should be in, IDK, utils or something?

larrybradley commented 8 years ago

The other centroid algorithms are currently in morphology.py, so you can put it there for now. I want to do some reorganization of that module (probably a new subpackage) at some point in the near future.

pllim commented 8 years ago

As decided during tag up today, it's a no-go.

pllim commented 8 years ago

Reopened issue based on the request from tag-up today to potentially add it just for its "centroiding algorithm" part.

@larrybradley , the code is in ofiltsky.py. Do you mean you only want the aptopt() function?