JSON Parser for Datalinks by d-giles · Pull Request #709 · astropy/pyvo

d-giles · 2025-11-21T22:11:51Z

As data becomes available from different cloud providers, some services are including cloud access information in the associated Datalink in JSON format.

This PR adds functionality to the DatalinkRecordMixin and the DatalinkResultsMixin to handle basic parsing of JSON entries in a uniform way. The methods ask for a column name and a key to identify specific entries in the JSON to parse and return, and allows matching other named parameters. The parsed JSON entries are returned in an astropy.Table with added columns to indicate the row of the original Results object to which the row belongs, and the row of the DatalinkResults where applicable.

While JSON entries are non-standard at present, they are not disallowed in DatalinkResults and are currently being included without official standard or guidance. This PR does not impose a standard, but provides a set of convenience functions which facilitate the use of existing data.

codecov · 2025-11-21T22:14:06Z

Codecov Report

❌ Patch coverage is 90.90909% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.71%. Comparing base (4180ae0) to head (34331bf).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
pyvo/dal/adhoc.py	90.90%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #709      +/-   ##
==========================================
+ Coverage   80.60%   80.71%   +0.11%     
==========================================
  Files          90       90              
  Lines       10366    10443      +77     
==========================================
+ Hits         8355     8429      +74     
- Misses       2011     2014       +3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

msdemlei

Thanks for taking pyVO on this presumably inevitable path. I think we can in principle merge this even before there is any proper agreement on how this should eventually look like.

However, I'd like it a lot better if there were not, for now, a default cloud provider; at least during the prototype phase, people should need to explicitly pass these in.

Also, I'd auto-detect columns based on an invented UCD (meta.ref.cloudstorage, say). If this turns out to work, we can always get it sanctioned. The alternative of using a utype ("adhoc:cloudstorage") could later interfer with obscore (though it really shouldn't). I'd not insist on that, though. If you say "no, I want bespoke column names", you'd have my blessing, too.

But most importantly: please make it crystal clear in the documentation that this is a draft API that is probably subject to change.

msdemlei

Sorry for having missed the review request. Not sure how it got lost.

From where I stand, I'm reasonably happy with what I see. I would prefer if someone from another place that uses "cloud" storage had a look at this and agreed this is the way to do it (I'm from Europe and hence am not afflicted); but if other maintainers are fine, too, I'll not stand in the way of merging.

In particular not if this then is accompanied with a short IVOA note that explains how this is supposed to work, how other data providers can adopt it and that shows a few pieces of code based on this.

…atic function

…sts, fixed changelog entry

bsipocz

Thanks @d-giles for the PR.

I left a lot of, mostly consistency related comments and a couple of cleanup suggestions, too.

If we can wrap this up in the next week or two, then I'll get it into the new release.

bsipocz · 2026-10-15T23:39:16Z

    >>> next(datalink.bysemantics("#this")).content_type
    'application/fits'

+Cloud Access Information [subject to change]


can we add a code example?

also, what do you mean subject to change? Add a note book in the narrative rather than in the heading instead.

bsipocz · 2026-10-15T23:44:26Z

+As data becomes available from different cloud providers, some services are
+including cloud access information in the associated Datalink in JSON format.
+The ``get_cloud_params`` and ``iter_get_cloud_params`` functions for
+Records and Results respectively return an ``astropy.Table`` with the


there is no such thing as astropy.Table.

Suggested change

Records and Results respectively return an ``astropy.Table`` with the

Records and Results respectively return an `~astropy.table.Table` with the

I don't think Records and Results are proper nouns, maybe rephrase this sentence, too? Maybe do an intersphinx link instead pointing to the class?

bsipocz · 2026-10-15T23:44:35Z

+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+As data becomes available from different cloud providers, some services are
+including cloud access information in the associated Datalink in JSON format.
+The ``get_cloud_params`` and ``iter_get_cloud_params`` functions for


Suggested change

The ``get_cloud_params`` and ``iter_get_cloud_params`` functions for

The ``get_cloud_params`` and ``iter_get_cloud_params`` methods for

bsipocz · 2026-10-15T23:47:01Z

+As data becomes available from different cloud providers, some services are
+including cloud access information in the associated Datalink in JSON format.


This reads a little bit weird/too vague. Could you maybe rephrase? Also, I believe we mostly use datalink as common noun in similar contexts above in this docs.

bsipocz · 2026-10-15T23:49:10Z

+Records and Results respectively return an ``astropy.Table`` with the
+parameters specified to access data via the cloud service provider specified.
+
+More generic functions, ``parse_json_params`` and ``iter_parse_json_params``,


Suggested change

More generic functions, ``parse_json_params`` and ``iter_parse_json_params``,

More generic methods, ``parse_json_params`` and ``iter_parse_json_params``,

also, consider intersphinx link all the methods here instead of just static highlighting

bsipocz · 2026-10-16T00:03:35Z

+
+        """
+
+        # init results table (avoiding adding import of astropy.table.Table)


I feel this is unnecessary premature optimization as we likely end up with Table imported anyway, but it can stay for now.

bsipocz · 2026-10-16T00:05:02Z

+            except UnboundLocalError:
+                pass


why do we expect this, can you add a comment for it?

bsipocz · 2026-10-16T00:09:53Z

+@pytest.fixture()
+def datalink_cloud1(mocker):
+    def callback(request, context):
+        dl_base = parse('pyvo/dal/tests/data/datalink/datalink.xml')


please use get_pkg_data_contents to get the datafile like we do elsewhere in the library

bsipocz · 2026-10-16T00:11:39Z

+    def get_cloud_params(
+        self,
+        provider: str,
+        colname: str = "cloud_access",


Suggested change

colname: str = "cloud_access",

*,

colname: str = "cloud_access",

bsipocz · 2026-10-16T00:15:13Z

+        """
+        for irow, dl_results in enumerate(self.iter_datalinks()):
+
+            products = dl_results.bysemantics("#this")


I feel this should reuse get_cloud_params rather than copy a large fraction of the same code

msdemlei requested changes Nov 24, 2025

View reviewed changes

Comment thread docs/dal/index.rst Outdated

d-giles force-pushed the cloud-info branch from d2848c5 to 2eb66b4 Compare December 6, 2025 23:40

bsipocz added this to the v1.9 milestone Dec 19, 2025

bsipocz added enhancement component: dal labels Dec 19, 2025

d-giles force-pushed the cloud-info branch from eedd4bd to 2afc229 Compare December 19, 2025 19:44

d-giles requested a review from msdemlei December 19, 2025 19:47

bsipocz removed this from the v1.9 milestone Jun 6, 2026

msdemlei approved these changes Jul 3, 2026

View reviewed changes

d-giles added 8 commits October 5, 2026 13:32

Added a json parser method to the Datalink mixins

f238dc3

Upated iter method to use iter_datalinks, added documentation and tests

8261f15

Upated documentation, removed duplicate code, fixed formatting

76d9fdb

docstring updates

6cc0b4a

added changelog entry

98c6e4f

added column guesser based on utype and ucd, changed parse_json to st…

310dd87

…atic function

Fixed handling of multiple entries for a single provider, improved te…

fed2d2e

…sts, fixed changelog entry

Formatting

34331bf

d-giles force-pushed the cloud-info branch from 2afc229 to 34331bf Compare October 5, 2026 17:36

bsipocz reviewed Oct 16, 2026

View reviewed changes

bsipocz added this to the v1.10.0 milestone Oct 16, 2026

	Records and Results respectively return an ``astropy.Table`` with the
	Records and Results respectively return an `~astropy.table.Table` with the

	The ``get_cloud_params`` and ``iter_get_cloud_params`` functions for
	The ``get_cloud_params`` and ``iter_get_cloud_params`` methods for

		As data becomes available from different cloud providers, some services are
		including cloud access information in the associated Datalink in JSON format.

	More generic functions, ``parse_json_params`` and ``iter_parse_json_params``,
	More generic methods, ``parse_json_params`` and ``iter_parse_json_params``,


		"""

		# init results table (avoiding adding import of astropy.table.Table)

	colname: str = "cloud_access",
	*,
	colname: str = "cloud_access",

Conversation

d-giles commented Nov 21, 2025

Uh oh!

codecov Bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

msdemlei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msdemlei left a comment

Choose a reason for hiding this comment

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Nov 21, 2025 •

edited

Loading