Skip to content

Commit

Permalink
Backing out of requesting alt_identifiers every time.
Browse files Browse the repository at this point in the history
This is supposed to address bug #522.  The code as it was up to here
would have needed aggregation of alt_identifiers (which are n:1 over
resources), or else we see duplicate capabilities.

But at least some registry operators prefer to not hit the rr.alt_identifier
table by default as long as it's not clear who will actually look at these
alternate identifiers.

But we maintain the alt identifiers in describe(); to do that, there's now
get_alternate_identifiers method returning these.

The downside: describe() now does an uncached network query.  Perhaps
we want to at least hide failures from there?  On the other hand, once
we are here we can also call get_contact() here; should we?
  • Loading branch information
msdemlei committed Feb 12, 2024
1 parent 3a24ac2 commit 2e8a828
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 19 deletions.
33 changes: 21 additions & 12 deletions pyvo/registry/regtap.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,7 @@ class RegistryResource(dalq.Record):
(f"\n ivo_string_agg(COALESCE(intf_role, ''), '{TOKEN_SEP}')",
"intf_roles"),
(f"\n ivo_string_agg(COALESCE(cap_description, ''), '{TOKEN_SEP}')",
"cap_descriptions"),
"alt_identifier"]
"cap_descriptions")]

def __init__(self, results, index, *, session=None):
dalq.Record.__init__(self, results, index, session=session)
Expand Down Expand Up @@ -599,14 +598,6 @@ def reference_url(self):
"""
return self.get("reference_url", decode=True)

@property
def alt_identifier(self):
"""Alternative identifier.
It is often used to provide the resource associated DOI.
"""
return self.get("alt_identifier", decode=True)

@property
def creators(self):
"""
Expand Down Expand Up @@ -1013,8 +1004,14 @@ def describe(self, *, verbose=False, width=78, file=None):
else:
print(f"Authors: {', '.join(creators[:nmax_authors])} et al.\n"
"See creators attribute for the complete list of authors.", file=file)
if self.alt_identifier:
print(f"Alternative identifier: {self.alt_identifier}", file=file)

alt_identifiers = self.get_alt_identifiers()
if alt_identifiers:
print(

Check warning on line 1010 in pyvo/registry/regtap.py

View check run for this annotation

Codecov / codecov/patch

pyvo/registry/regtap.py#L1008-L1010

Added lines #L1008 - L1010 were not covered by tests
"Alternative identifier(s): {}".format(
", ".join(alt_identifiers)),
file=file)

if self.reference_url:
print("More info: " + self.reference_url, file=file)

Expand Down Expand Up @@ -1043,6 +1040,18 @@ def get_contact(self):

return "\n".join(contacts)

def get_alt_identifiers(self):
"""return a sequence of non-ivoid identifiers for the resource.
This is typically used to provide a DOI for the resource.
"""
res = get_RegTAP_service().run_sync("""

Check warning on line 1048 in pyvo/registry/regtap.py

View check run for this annotation

Codecov / codecov/patch

pyvo/registry/regtap.py#L1048

Added line #L1048 was not covered by tests
SELECT alt_identifier
FROM rr.alt_identifier
WHERE ivoid={}""".format(
rtcons.make_sql_literal(self.ivoid)))
return [r["alt_identifier"] for r in res]

Check warning on line 1053 in pyvo/registry/regtap.py

View check run for this annotation

Codecov / codecov/patch

pyvo/registry/regtap.py#L1053

Added line #L1053 was not covered by tests

def _build_vosi_column(self, column_row):
"""
return a io.vosi.vodataservice.Column element for a
Expand Down
14 changes: 13 additions & 1 deletion pyvo/registry/tests/test_regtap.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,15 @@ def test_get_contact():
" <gavo@ari.uni-heidelberg.de>")


@pytest.mark.remote_data
def test_get_alt_identifier():
rsc = _makeRegistryRecord(ivoid="ivo://cds.vizier/i/337")
assert set(rsc.get_alt_identifiers()) == {
'doi:10.26093/cds/vizier.1337',
'bibcode:doi:10.5270/esa-ogmeula',
'bibcode:2016yCat.1337....0G'}


@pytest.mark.remote_data
class TestDatamodelQueries:
# right now, the data model queries are all rather sui generis, and
Expand Down Expand Up @@ -754,6 +763,7 @@ def test_unique_standard_id(self):
intf_roles=["std"])
assert rsc.standard_id == "ivo://ivoa.net/std/tap"

@pytest.mark.remote_data
def test_describe_multi(self, flash_service):
out = io.StringIO()
flash_service.describe(verbose=True, file=out)
Expand All @@ -765,9 +775,10 @@ def test_describe_multi(self, flash_service):
assert "Multi-capability service" in output
assert "Source: 1996A&A...312..539S" in output
assert "Authors: Wolf" in output
assert "Alternative identifier: doi:10.21938/" in output
assert "Alternative identifier(s): doi:10.21938/" in output
assert "More info: http://dc.zah" in output

@pytest.mark.remote_data
def test_describe_long_authors_list(self):
"""Check that long list of authors use et al.."""
rsc = _makeRegistryRecord(
Expand All @@ -785,6 +796,7 @@ def test_describe_long_authors_list(self):
# output should cut at 5 authors
assert "Authors: a, a, a, a, a et al." in output

@pytest.mark.remote_data
def test_describe_long_author_name(self):
"""Check that long author names are truncated."""
rsc = _makeRegistryRecord(
Expand Down
9 changes: 3 additions & 6 deletions pyvo/registry/tests/test_rtcons.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,7 @@ def test_expected_columns(self):
"\n ivo_string_agg(COALESCE(standard_id, ''), ':::py VO sep:::') AS standard_ids, "
"\n ivo_string_agg(COALESCE(intf_type, ''), ':::py VO sep:::') AS intf_types, "
"\n ivo_string_agg(COALESCE(intf_role, ''), ':::py VO sep:::') AS intf_roles, "
"\n ivo_string_agg(COALESCE(cap_description, ''), ':::py VO sep:::') AS cap_descriptions, "
"alt_identifier")
"\n ivo_string_agg(COALESCE(cap_description, ''), ':::py VO sep:::') AS cap_descriptions")

def test_group_by_columns(self):
# Again, this will break as regtap.RegistryResource.expected_columns
Expand All @@ -480,8 +479,7 @@ def test_group_by_columns(self):
"source_format, "
"source_value, "
"region_of_regard, "
"waveband, "
"alt_identifier"))
"waveband"))

def test_joined_tables(self):
expected_tables = [
Expand All @@ -491,7 +489,6 @@ def test_joined_tables(self):
"rr.resource",
"rr.capability",
"rr.interface",
"rr.alt_identifier"
]
assert all(table in _build_regtap_query_with_fake([rtcons.Author("%Hubble%")])
for table in expected_tables)
Expand Down Expand Up @@ -519,4 +516,4 @@ def test_all_constraints():
'reference_url', 'creator_seq', 'created', 'updated',
'rights', 'content_type', 'source_format', 'source_value',
'region_of_regard', 'waveband', 'access_urls', 'standard_ids',
'intf_types', 'intf_roles', 'cap_descriptions', 'alt_identifier')
'intf_types', 'intf_roles', 'cap_descriptions')

0 comments on commit 2e8a828

Please sign in to comment.