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 query_target methods to query_object be consistent with the rest of the API #3217

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

Comments

@bsipocz
Copy link
Member

bsipocz commented Feb 20, 2025

I've noticed the API inconsistency in esa.hubble and esa.jwst to use the name query_target as opposed to what we do in the rest of the package.

I suppose we could rename (with appropriately long deprecation for the old names) to make the API more consistent.

cc @jespinosaar

@jespinosaar
Copy link
Contributor

Hi @bsipocz ! could you please expand on this issue? I mean, I am assuming we should rename the query_target method, which would be the method name we should use?
Thanks!

@jespinosaar
Copy link
Contributor

I can see maybe it is query_object, isn't it?

@bsipocz
Copy link
Member Author

bsipocz commented Feb 20, 2025

Yes, query_object. Now, I haven't dived into understanding if target would mean something broader or not, just opened the issue while it's fresh (I'm in the middle of the Euclid review and opening side issues I run into during that review)

@bsipocz
Copy link
Member Author

bsipocz commented Feb 20, 2025

Also, if there is no objection to the rename, I can do this as part of my upcoming API cleanup and consolidating efforts.

@bsipocz
Copy link
Member Author

bsipocz commented Feb 20, 2025

Similarly, cone_search may need to be renamed query_region.

@jespinosaar
Copy link
Contributor

Also, if there is no objection to the rename, I can do this as part of my upcoming API cleanup and consolidating efforts.

Sure, you can do it but please, we should keep in mind backwards compatibility, so maybe we should include a warning in query_target and keep it for some time...

@bsipocz
Copy link
Member Author

bsipocz commented Feb 24, 2025

so maybe we should include a warning in query_target and keep it for some time...

Definitely. Any renaming should be done as deprecations that are being kept around for a long time (I would think at least a year, or maybe more)

@jespinosaar
Copy link
Contributor

Also, if there is no objection to the rename, I can do this as part of my upcoming API cleanup and consolidating efforts.

Sure! you can do it if you have time. If not, please let me know and I will include it in our planning.

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

No branches or pull requests

2 participants