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

Refactor sampling functions #126

Closed
ismael-mendoza opened this issue Apr 7, 2021 · 1 comment
Closed

Refactor sampling functions #126

ismael-mendoza opened this issue Apr 7, 2021 · 1 comment
Assignees
Labels
brainstorm ideas question Further information is requested refactor code refactoring
Milestone

Comments

@ismael-mendoza
Copy link
Collaborator

ismael-mendoza commented Apr 7, 2021

It occurred to me that given how we already standardized the catalog information in catalog.py, we really don't need another layer of complexity in the sampling functions. I believe that only two sampling 'modes' are required to cover all possible cases:

  • 'random': Create random blends based on the catalog of size (1, max_sources)
  • 'group': Use the group_id column in the catalog to create blends. We can take the average ra, dec and use these as the center of the postage stamp.

If we really wanted to we can provide utility functions to allow the user to make cuts in their catalog (but I think users should know their catalog and can make cuts if they want to). I don't think 'global cuts' should be the job of the sampling functions.

In terms of adding a group_id column, I do think it's worth providing utility functions for doing that based on the most common use cases. Along the lines of #16

If this works out the draw_blend_generator will no longer take in a sampling function, just a flag with value either random or group. Further simplifying the UI

@ismael-mendoza ismael-mendoza added brainstorm ideas refactor code refactoring labels Apr 7, 2021
@ismael-mendoza ismael-mendoza self-assigned this Apr 7, 2021
@ismael-mendoza ismael-mendoza added this to the v1.0.0 milestone May 20, 2021
@ismael-mendoza ismael-mendoza added the question Further information is requested label Jun 15, 2021
@thuiop thuiop removed this from the v1.0.0 milestone Jul 28, 2021
@ismael-mendoza ismael-mendoza added this to the 1.1.0 milestone Sep 12, 2022
@ismael-mendoza
Copy link
Collaborator Author

actually like the structure in the newly refactored version. Adding something with group_id sounds like a new feature at this point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brainstorm ideas question Further information is requested refactor code refactoring
Projects
None yet
Development

No branches or pull requests

2 participants