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

MAINT: rename cone_search methods to query_region #3219

Open
bsipocz opened this issue Feb 21, 2025 · 8 comments
Open

MAINT: rename cone_search methods to query_region #3219

bsipocz opened this issue Feb 21, 2025 · 8 comments

Comments

@bsipocz
Copy link
Member

bsipocz commented Feb 21, 2025

No description provided.

@jespinosaar
Copy link
Contributor

Hi @bsipocz ! I am discussing this together with the Euclid team and our scientists. It is true that query_region is more generic, but cone_search is a common method in astronomical community, and even a standard in VO, so maybe we could keep it for specific cone_searches (coordinates + radius). Regarding query_region, maybe it is more associated to squared regions. What do you think?

@keflavich
Copy link
Contributor

query_region has been standard in astroquery from its inception, and takes on the general meaning of 'query any region on the sky'. In practice, most implementations are cone_search, but there are some that also allow box searches, and in the ideal world everyone would allow arbitrary region specification.

I would be happy if cone_search is retained, but query_region is added as a wrapper. If you also support box (rectangle or square) searches, or other region shapes, those should also be supported in query_region

@keflavich
Copy link
Contributor

OK I was wrong, it took us several years to come to query_region, and it didn't end up as cone_search because we were supporting a lot of image cutout services, not a lot of catalogs. Interesting to look back on those old conversations...

@bsipocz
Copy link
Member Author

bsipocz commented Feb 24, 2025

Nevertheless, a lot of underlying machinery relies on TAP rather than SCS, and some supports cone/box/all-sky an a spatial constraint, so at this point having only cone search is not preferred. So even is those other spatial queries won't be supported, a wrapper will be needed so we have API consistency within astroquery itself.

@jespinosaar
Copy link
Contributor

Hi @bsipocz. We are coordinating with Euclid team as well regarding the changes in the names. We are adding to our planning the creation of the the wrappers, together with other module migrations to PyVO. I will let you know when we apply these changes. Thanks!

@bsipocz
Copy link
Member Author

bsipocz commented Feb 27, 2025

Oh, wow, lovely to hear about the PyVO plans!

@jespinosaar
Copy link
Contributor

You know, in the end we need to do it step by step. We started with a new module. Now that we have a reference, we are starting migrating the most simple ones and adding more features. And then we will start with the most complex ones, the ones that have specific features on TapUtils, to use modern libraries and code. Maybe, at some point, it is interesting to include some of this new code in PyVO library itself (e.g. login features, we can test the current implementation of tap_upload from PyVO...).

@bsipocz
Copy link
Member Author

bsipocz commented Feb 27, 2025

It will be a long process. And indeed, all the extra functionality of tapplus would be nice to make its way upstream into pyvo. Dice we have the prototype functionality it's finally not a blocker if something is not in the standards yet.

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