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

API discussion for list_collections #3077

Open
bsipocz opened this issue Aug 1, 2024 · 4 comments
Open

API discussion for list_collections #3077

bsipocz opened this issue Aug 1, 2024 · 4 comments

Comments

@bsipocz
Copy link
Member

bsipocz commented Aug 1, 2024

Now that I'm adding SSA methods, it becomes clear that some of the arguments and functionality is named very similarly as those of SIA's. Namely we work with collections.

So, the question would be: which one do you find more user friends? Adding new methods, e.g. list_collections_ssa (and then also rename the currently existing list_collections to list_collections_sia, or instead add a kwarg that is either sia or ssa, and defaulting it to sia to keep the current behaviour?

cc @andamian and @keflavich

(Currently this affects the IRSA module, but ultimately will affect other ones as we turn to use SIAv2 and SSA backends instead of the webforms)

@keflavich
Copy link
Contributor

I am very slightly in favor of list_collections_{sia,ssa} because of improved discoverability (tab-completion will show those). But I am also perfectly happy with the other option.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 1, 2024

That also means to deprecate the current list_collections() method.

@andamian
Copy link

andamian commented Aug 8, 2024

list_collections is pretty basic right now. I would use optional filtering arguments at the expense of discoverability to leave the door open for other potential orthogonal filters.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 8, 2024

I'm even more inclined to doing the list_collections() way, as it seems that we'll get all collections combined with a simple CAOM search, so will in fact need to do more filtering to get either SIA or SSA. And who knows, maybe there will be use cases where having the combined list is useful.

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

3 participants