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

Added target_by_sdss_id_list endpoint #13

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

mtaghiza
Copy link
Contributor

@mtaghiza mtaghiza commented Feb 15, 2024

This PR adds a new post request endpoint to get targets by a list of sdss_ids, using a body parameter. This is for when we add support for uploading a file to the front end. Updates the get_targets_by_sdss_id endpoint to accept a list of sdss_ids.

@mtaghiza mtaghiza requested review from havok2063 and removed request for havok2063 February 15, 2024 14:21
Comment on lines 175 to 196
def get_targets_by_sdss_id_list(sdss_id_list: list = []) -> peewee.ModelSelect:
""" Perform a search for SDSS targets on vizdb.SDSSidStacked based on a list if sdss_ids.

Perform a search for SDSS targets from a list of sdss_id values,
using the peewee ORM in the vizdb.SDSSidStacked table.
We return the peewee ModelSelect directly here so it can be
easily combined with other queries, if needed.

In the route endpoint itself, remember to return wrap this in a list.

Parameters
----------
sdss_id_list : List[int]
list of sdss_id values

Returns
-------
peewee.ModelSelect
the ORM query
"""

return vizdb.SDSSidStacked.select().where(vizdb.SDSSidStacked.sdss_id.in_(sdss_id_list))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function isn't that different than the existing get_targets_by_sdss_id. Can you consolidate these two into a single function.

Copy link
Contributor

@havok2063 havok2063 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment about consolidating similar functions.

@havok2063
Copy link
Contributor

@mtaghiza can you edit the description of the PR with what this is, for transparency and posterity?

@havok2063 havok2063 added the enhancement New feature or request label Mar 12, 2024
@havok2063 havok2063 merged commit dcf8ab6 into main Mar 12, 2024
2 checks passed
@havok2063 havok2063 deleted the target_by_sdss_id_list branch June 7, 2024 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants