From 095b734447cbf65ef40d81f0712109a89a571cc9 Mon Sep 17 00:00:00 2001 From: Markus Demleitner Date: Mon, 12 Feb 2024 14:31:25 +0100 Subject: [PATCH 1/2] Backing out of requesting alt_identifiers every time. 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? --- CHANGES.rst | 4 ++++ pyvo/registry/regtap.py | 33 +++++++++++++++++++----------- pyvo/registry/tests/test_regtap.py | 14 ++++++++++++- pyvo/registry/tests/test_rtcons.py | 9 +++----- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 50caa800e..cc39510c6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -20,6 +20,10 @@ Enhancements and Fixes - registry.Ivoid now accepts multiple ivoids and will then match any of them. [#517] +- Backing out of having alt_identifier in RegistryResource throughout. + Use get_alt_identifier() instead [#523] + + Deprecations and Removals ------------------------- diff --git a/pyvo/registry/regtap.py b/pyvo/registry/regtap.py index 860c02eca..ae4e3b134 100644 --- a/pyvo/registry/regtap.py +++ b/pyvo/registry/regtap.py @@ -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) @@ -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): """ @@ -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( + "Alternative identifier(s): {}".format( + ", ".join(alt_identifiers)), + file=file) + if self.reference_url: print("More info: " + self.reference_url, file=file) @@ -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(""" + SELECT alt_identifier + FROM rr.alt_identifier + WHERE ivoid={}""".format( + rtcons.make_sql_literal(self.ivoid))) + return [r["alt_identifier"] for r in res] + def _build_vosi_column(self, column_row): """ return a io.vosi.vodataservice.Column element for a diff --git a/pyvo/registry/tests/test_regtap.py b/pyvo/registry/tests/test_regtap.py index 089c0e069..c00b69913 100644 --- a/pyvo/registry/tests/test_regtap.py +++ b/pyvo/registry/tests/test_regtap.py @@ -720,6 +720,15 @@ def test_get_contact(): " ") +@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 @@ -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) @@ -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( @@ -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( diff --git a/pyvo/registry/tests/test_rtcons.py b/pyvo/registry/tests/test_rtcons.py index 35345af17..eddf923a7 100644 --- a/pyvo/registry/tests/test_rtcons.py +++ b/pyvo/registry/tests/test_rtcons.py @@ -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 @@ -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 = [ @@ -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) @@ -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') From 5609504c49cf3cf0f034f4130c3c346db575a6cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brigitta=20Sip=C5=91cz?= Date: Thu, 15 Feb 2024 23:11:31 -0800 Subject: [PATCH 2/2] MAINT: making linter happy. --- pyvo/registry/regtap.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyvo/registry/regtap.py b/pyvo/registry/regtap.py index ae4e3b134..ebd5c4b85 100644 --- a/pyvo/registry/regtap.py +++ b/pyvo/registry/regtap.py @@ -1048,8 +1048,7 @@ def get_alt_identifiers(self): res = get_RegTAP_service().run_sync(""" SELECT alt_identifier FROM rr.alt_identifier - WHERE ivoid={}""".format( - rtcons.make_sql_literal(self.ivoid))) + WHERE ivoid={}""".format(rtcons.make_sql_literal(self.ivoid))) return [r["alt_identifier"] for r in res] def _build_vosi_column(self, column_row):