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

ENH: pyvo.registry.regtap.RegistryResource.access_modes to return sorted set #334

Closed
bsipocz opened this issue Jun 1, 2022 · 3 comments
Closed

Comments

@bsipocz
Copy link
Member

bsipocz commented Jun 1, 2022

Follow-up for #289: having a deterministic order would be nice.

See: scientific-python/pytest-doctestplus#182

@msdemlei
Copy link
Contributor

msdemlei commented Jun 9, 2022

[Sent this in by mail on 2022-06-02; not sure why it didn't make it here]

I see... on the other hand, the set this function returns right now is a very appropriate representation to what this is: A set of ways a service can be accessed. Being frank here is relevant because, for instance, it clearly states that there are never duplicates in what is returned (where a resource may have multiple capabilities with the same standardID).

And of course the natural operation on the result is asking "is there a TAP service in here?", whereas "what's the position of the TAP service?" is utterly meaningless.

Making things easily testable is one thing, but we shouldn't forsake appropriate data structures for that; pllim's argument over at the doctestplus bug would mean set-returning functions are out, which seems wrong to me (but then I give them it's not entriely trivial to fix given the way doctests work; perhaps they should have a

  +PARSE_USING: set({})

modifier that would let you say "compare stuff not after serialisation, but de-serialise the result literal and compare then"?).

If this were a normal doctest, I'd fix it by writing list(sorted(res.access_modes())).

That, however, is not an option for actual documentation, as that would defeat its purpose (at least here).

Frankly: I'd say the +IGNORE_OUTPUT feels really rational to me here.

@bsipocz
Copy link
Member Author

bsipocz commented Jun 9, 2022

Yes, I feel you're right. There are a few shortcomings of the doctesting, I will raise it with others as this may be a limitation for other packages, too.

@msdemlei
Copy link
Contributor

msdemlei commented Oct 11, 2022 via email

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