Adding some documentation on pyvo.samp #558
Conversation
|
That's nice, but maybe the file should be in a folder, either Edit: And I guess there is no way we could have the example work in doctests because it requires a samp hub to be up? |
|
On Wed, Jun 26, 2024 at 12:38:38AM -0700, Manon Marchand wrote:
That's nice, but maybe the file should be in a folder, either
`utils` or its own?
Our systematics so far is that we reflect the organisation of the
source code in docs, and samp.py, for obscure historical reasons, has
been top-level.
So, as long as that's the case, I'd say the samp.rst should be
top-level in docs, too. I suppose this would change when samp comes
in from astropy (btw, is anyone actively pursuing it? Should we try
to find someone?). If you are really concerned about a file in the
top level, it could become samp/index.rst, anticipating the samp
move, but I'd find that a bit odd at this point.
|
Makes sense, we can move it later when this is merged with the astropy samp module |
Afaik no one is actively pursuit it besides the abandoned draft PR that we have it for here. I'm not sure how much work it would be for a maintainer here to take over that PR and wrap it up (the original author has switched jobs and is no longer a contractor for astropy) |
There was a problem hiding this comment.
The only downside I see here is that this makes the samp API public (strictly speaking), as it hasn't been included in the documentation so far.
So, if we eventually make the astropy.samp move, that may come with deprecations.
OTOH, this is very much a technicality, as even without samp documentation, it may be used out in the wild, so any changes may have to be done by depreactions, too.
So, with this in mind, I'll ping @andamian and @tomdonaldson as well to sign off on adding the samp API to the docs.
|
What is driving this change? Shouldn't be done after the |
|
On Tue, Jul 02, 2024 at 10:35:18AM -0700, Adrian wrote:
What is driving this change? Shouldn't be done after the `samp`
port over? It's true that the port work is long overdue and there's
no clear path forward but that seems to be the logical progression,
unless there's an urgency to have this documentation out earlier.
Well, the code has been in for a long time, the API kind of makes
sense, and I'm using it quite extensively in my VO course
(https://codeberg.org/msdemlei/pyvo-course).
Let me quote from the Zen of Python:
Now is better than never.
Although never is often better than *right* now.
Is there a serious reason to suspect this is a case of line two
rather than line one?
|
I hear you. So we have an API that's working and it's being used. We just commit to it and try to minimize changes (deprecations) when we port over SAMP. I'm all good with that. |
|
Yeap, even though it wasn't officially part of the API, it's been around for a long while now with people possibly using it, so even if we technically could change it without saying anything, we shouldn't really do that. So with that, I agree that it's better to document what we have now, and then think about the porting (which we have done for a few years now, so who knows when it will actually happen). |
Adding some documentation on pyvo.samp
This has been missing so far, which is a bit of a pity given that these are fairly useful functions.
I realise we may move over ex pysamp to here at some point, and that might put the .connection() context manager discussed here into question. But then whether we have this bit of documentation or not, we cannot just break it.