-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
BUG: duplicated class names #399
Comments
On Fri, Jan 13, 2023 at 09:24:25PM -0800, Brigitta Sipőcz wrote:
In the case of SIA and SIAv2 `SIAService`, `SIAQuery`, `SIAResults` are shared names between the two services. However, they are not compatible, and in practice SIAv2 is totally missing from the documentation.
I would suggest to rename the classes in siav2.py (with a deprecation for the current names), or come up with a way that enables someone to nicely use both SIA and SIAv2 in the same code.
+1 for renaming. Disclaimer: Since I've argued strongly against
SIAv2 and still think it shouldn't have been written, I probably
won't be able to get myself to actually writing the code.
|
Isn't SIA2 supposed to eventually supersede SIA? Wasn't that the reason for v2? |
I'm happy to make the PR for this, once there is an agreement. IMO now that both SIA and v2 exists, they should both be supported in pyvo as there are archives that have datasets for either. I'm ok if v2 eventually overtakes, and then think that maybe the renaming should go the other way. Or making SIA private and accessible with a kwarg (but that would be a serious API change). |
On Mon, Jan 16, 2023 at 06:12:43PM -0800, Brigitta Sipőcz wrote:
I'm happy to make the PR for this, once there is an agreement.
Oh, and on de-duplicating the class names: Brigitta, I'm absolutely
in favour of going ahead with this. We'll want that regardless of
how we fix the discovery part. Thanks for the offer!
|
On Mon, Jan 16, 2023 at 06:12:43PM -0800, Brigitta Sipőcz wrote:
I'm happy to make the PR for this, once there is an agreement.
IMO now that both SIA and v2 exists, they should both be supported
in pyvo as there are archives that have datasets for either. I'm
Well, for the forseeable future we will have both SIA1 and SIA2, and
I don't see a strong push to obsolete SIA1. DaCHS, for one, doesn't
do any pushing so far (actually, we still tell people to do SIA1
services, where SIA2 is basically just a shell around obscore).
The underlying problem is interesting, as it is the first time we
have a major version jump in a protocol that is widely used, and it
is an interesting question how to handle this. Perhaps we should
schedule a session on this at an Interop (and/or take this to the DAL
list) -- we've had some interesting discussion on this at a EuroVO
tech forum in the connection with SCS, where SCS2 was in the air for
a while (it's not any more now that modern VOTable is legal in SCS1).
For pyVO discovery, I *think* there's a reasonable way forward:
(a) we add a servicetype sia2.
(b) we make "image" match sia1 and sia2 and magic things in some way
(I *think* some logic in get_interface should do the trick) that if
there is exactly sia1 and sia2 capabilities (and no others) on a
service, no ValueError is raised because of ambiguous capabilities
but instead sia2 is returned.
To make this work transparently to the user, we'd have to make sia2's
query method accept a superset of the arguments of sia1's query() and
then say the sia1.query API is what people can rely on when doing
all-VO image queries.
Does anyone see a major hitch in this?
|
In the case of SIA and SIAv2
SIAService
,SIAQuery
,SIAResults
are shared names between the two services. However, they are not compatible, and in practice SIAv2 is totally missing from the documentation.I would suggest to rename the classes in siav2.py (with a deprecation for the current names), or come up with a way that enables someone to nicely use both SIA and SIAv2 in the same code.
(This came up during investigating the missing services in the registry search reported in #397)
The text was updated successfully, but these errors were encountered: