Skip to content
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

_colors and psflike #37

Closed
rainwoodman opened this issue Mar 8, 2016 · 4 comments
Closed

_colors and psflike #37

rainwoodman opened this issue Mar 8, 2016 · 4 comments

Comments

@rainwoodman
Copy link
Contributor

Currently we have a few functions with _colors, a few functions without _colors and some include psflike, some do not.

It does look a bit messy especially as people start to depend on exactly which cut includes which in downstream code.

@sbailey
Copy link
Contributor

sbailey commented Mar 8, 2016

Context: the _colors() functions are used by the desisim template simulations to make sure that the templates have the right colors, while not mucking around with things like whether they are PSF-like or not. One option to clarify the situation might be to standardize on names like:

  • full cuts: isLRG(), isELG(), isBGS() etc.
  • utility subsets: has_LRG_colors(), has_MWS_STAR_colors() etc.

We could also update top-level package documentation to clarify what is what.

@rainwoodman
Copy link
Contributor Author

Or maybe collecting functions of the same type into a single namespace, e.g.

isLRG.colors()
isLRG.full() -- probably isLRG()
isLRG.psf()

@moustakas
Copy link
Member

+1 on @rainwoodman's suggestion but with a little twist. Moving the functionality into a single Class would be a nice way to also incorporate alternative / exploratory / non-official selection criteria for each target class. For example, we may want to do a full selection of, e.g., LRGs using expanded color boundaries. As long as (1) the implemented functions are well-documented by the Target Selection working group and (2) the function used to apply the cuts is propagated into the output catalogs (see #20) then this could work.

@moustakas
Copy link
Member

I'm going to close this for now because the functionality we have coded up has been relatively stable for some time, and we also have a new 'sandbox' area for algorithms in development (see #143). We can revisit whether we need a dedicated Class for each spectral/target type at a later date if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants