Feature/mx 1677 orcid extractor #361

vyvytranngoc · 2025-01-09T15:58:23Z

PR Context

connector in mex-common that takes a string or orcid ID as argument
returns orcid person
convenience function: search for person, if one is found, transform to ExtractedPerson

Added

Connector class for retrieving ORCID data by ID or name
methods for extracting data from orcid
methods to transform from OcidPerson to mex person
model class for orcid data
unit tests

Changes

Deprecated

Removed

Fixed

Security

…mmon into feature/mx-1677-orcid-extractor

…nished)

…mmon into feature/mx-1677-orcid-extractor

vyvytranngoc · 2025-01-09T16:03:30Z

FAILED tests/primary_source/test_extract.py::test_extract_seed_primary_sources - AssertionError: assert 5 == 4
 +  where 5 = len([SeedPrimarySource(identifier='mex', title=[Text(value='Metadata Exchange', language=<TextLanguage.EN: 'en'>)]), SeedPrimarySource(identifier='organigram', title=[Text(value='Organizational Units', language=<TextLanguage.EN: 'en'>)]), SeedPrimarySource(identifier='ldap', title=[Text(value='Active Directory', language=<TextLanguage.EN: 'en'>)]), SeedPrimarySource(identifier='wikidata', title=[Text(value='Wikidata APIs', language=<TextLanguage.EN: 'en'>)]), SeedPrimarySource(identifier='orcid', title=[Text(value='Open Researcher Contributor Identification Initiative', language=<TextLanguage.EN: 'en'>)])])

I think this occurs because I have added orcid as a primary source?

cutoffthetop · 2025-01-10T13:20:34Z

~~tests/orcid/model.yaml seems to be empty, did you have plans for that?~~ done ✔️

cutoffthetop · 2025-01-10T13:22:11Z

FAILED tests/primary_source/test_extract.py::test_extract_seed_primary_sources - AssertionError: assert 5 == 4
I think this occurs because I have added orcid as a primary source?

yep, that's it. feel free to bump the expected length up to 5

cutoffthetop

good work already! i found some unused bits and pieces, and i had one bigger issue with the way the orcid models are written. could you take a look at that first?

i didn't review the tests yet, but i think they might change a bit anyway once the models are updated.

mex/common/settings.py

CHANGELOG.md

mex/common/orcid/connector.py

mex/common/orcid/transform.py

mex/common/orcid/extract.py

mex/common/orcid/transform.py

mex/common/orcid/models/person.py

mex/common/orcid/extract.py

…mmon into feature/mx-1677-orcid-extractor

CHANGELOG.md

mex/common/orcid/extract.py

mex/common/orcid/connector.py

mex/common/orcid/extract.py

mex/common/orcid/transform.py

mex/common/orcid/models/person.py

cutoffthetop

it's coming along! we just need to ensure proper unit test isolation

mex/common/orcid/connector.py

mex/common/settings.py

tests/orcid/test_connector.py

tests/orcid/conftest.py

…mmon into feature/mx-1677-orcid-extractor

vyvytranngoc · 2025-01-30T15:32:13Z

I was running my unit tests after pulling all new changes from main. But now I encounter this error:

mex\common\types\vocabulary.py:71: in <listcomp>
    Concept.model_validate(raw_vocabulary)
E   pydantic_core._pydantic_core.ValidationError: 1 validation error for Concept
E   altLabel
E     Input should be a valid list [type=list_type, input_value={'de': 'RESTful API', 'en': 'RESTful API'}, input_type=dict]
E       For further information visit https://errors.pydantic.dev/2.9/v/list_type

vyvytranngoc · 2025-01-30T15:33:22Z

But I fixed the requested changes and made sure I'm running unit tests and no integration tests

…mmon into feature/mx-1677-orcid-extractor

cutoffthetop

a few simplifications and were good to go

cutoffthetop · 2025-02-04T15:58:15Z

mex/common/orcid/connector.py

+    @staticmethod
+    def get_data_by_id(orcid_id: str) -> dict[str, Any]:
+        """Retrieve data by UNIQUE ORCID ID.
+
+        Args:
+            orcid_id: Unique identifier in ORCID system.
+
+        Returns:
+            Personal data of the single matching id.
+        """
+        orcidapi = OrcidConnector.get()
+        # or endpoint = f"{orcid_id}/person"
+        endpoint = f"{orcid_id}/record"
+        return dict(orcidapi.request(method="GET", endpoint=endpoint))


why does this have to be static?

Suggested change

@staticmethod

def get_data_by_id(orcid_id: str) -> dict[str, Any]:

"""Retrieve data by UNIQUE ORCID ID.

Args:

orcid_id: Unique identifier in ORCID system.

Returns:

Personal data of the single matching id.

"""

orcidapi = OrcidConnector.get()

# or endpoint = f"{orcid_id}/person"

endpoint = f"{orcid_id}/record"

return dict(orcidapi.request(method="GET", endpoint=endpoint))

def get_data_by_id(self, orcid_id: str) -> dict[str, Any]:

"""Retrieve data by UNIQUE ORCID ID.

Args:

orcid_id: Unique identifier in ORCID system.

Returns:

Personal data of the single matching id.

"""

endpoint = f"{orcid_id}/record"

return dict(self.request(method="GET", endpoint=endpoint))

cutoffthetop · 2025-02-04T16:01:11Z

mex/common/orcid/connector.py

+        response = self._send_request("HEAD", url=url, params={})
+        response.raise_for_status()
+
+    def check_orcid_id_exists(self, orcid_id: str) -> bool:


this method is unused in mex-common, do you need it for mex-backend?

cutoffthetop · 2025-02-04T16:02:45Z

mex/common/orcid/connector.py

+
+    def fetch(self, filters: dict[str, Any]) -> dict[str, Any]:
+        """Perform a search query against the ORCID API."""
+        query = OrcidConnector.build_query(filters)


Suggested change

query = OrcidConnector.build_query(filters)

query = self.build_query(filters)

cutoffthetop · 2025-02-04T16:03:28Z

mex/common/orcid/connector.py

+        return dict(orcidapi.request(method="GET", endpoint=endpoint))
+
+    @staticmethod
+    def get_data_by_name(


same here, why make this static?

instead of orcidapi = OrcidConnector.get() you could just use self

cutoffthetop · 2025-02-04T16:07:45Z

mex/common/testing/plugin.py

+
+    def build_query(filters: dict[str, Any]) -> str:
+        """Construct the ORCID API query string."""
+        if "givennames" in filters:
+            return "givennames:Josiah AND familyname:Carberry"
+        return "given-names:Josiah AND family-name:Carberry"
+
+    monkeypatch.setattr(OrcidConnector, "build_query", staticmethod(build_query))


Suggested change

def build_query(filters: dict[str, Any]) -> str:

"""Construct the ORCID API query string."""

if "givennames" in filters:

return "givennames:Josiah AND familyname:Carberry"

return "given-names:Josiah AND family-name:Carberry"

monkeypatch.setattr(OrcidConnector, "build_query", staticmethod(build_query))

you don't need to mock this method. since it does not do any network-io, it is "unit-test-safe"

cutoffthetop · 2025-02-04T16:09:47Z

mex/common/orcid/extract.py

+    Returns:
+        Orcidrecord of the matching person by name.
+    """
+    orcid_data = OrcidConnector.get_data_by_name(


when you make get_data_by_name non-static, this would become:

Suggested change

orcid_data = OrcidConnector.get_data_by_name(

orcid_data = OrcidConnector.get().get_data_by_name(

vyvytranngoc added 12 commits October 31, 2024 12:02

started orcid extractor

3e392cf

working OrcidConnector (name and orcidId) with running tests

20d9727

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

82a859d

…mmon into feature/mx-1677-orcid-extractor

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

57cd6ac

…mmon into feature/mx-1677-orcid-extractor

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

06040ad

…mmon into feature/mx-1677-orcid-extractor

Extended OrcidPerson class, added extract and transform methods (unfi…

fa27e0e

…nished)

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

136c725

…mmon into feature/mx-1677-orcid-extractor

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

67ba02b

…mmon into feature/mx-1677-orcid-extractor

finished extract methods for orcid

90a2b65

Finished orcid connector

9f7f13e

Finished orcid connector

b39c714

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

d86029b

…mmon into feature/mx-1677-orcid-extractor

RKIMetadataExchange assigned vyvytranngoc Jan 9, 2025

Rmoved dummy file

19fa4a4

cutoffthetop requested changes Jan 10, 2025

View reviewed changes

vyvytranngoc added 4 commits January 16, 2025 16:52

fixed requsted changes and remodeled OrcidPerson class to OrcidRecord

93d50c3

moved get_data_by_id from extract to connector

781ccaa

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

74c0346

…mmon into feature/mx-1677-orcid-extractor

removed get_data_by_name from extract to connector

bef1ee6

cutoffthetop reviewed Jan 27, 2025

View reviewed changes

cutoffthetop requested changes Jan 27, 2025

View reviewed changes

mex/common/orcid/connector.py Outdated Show resolved Hide resolved

mex/common/settings.py Outdated Show resolved Hide resolved

tests/orcid/test_connector.py Outdated Show resolved Hide resolved

tests/orcid/conftest.py Outdated Show resolved Hide resolved

vyvytranngoc added 4 commits January 28, 2025 09:59

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

3ca8a6d

…mmon into feature/mx-1677-orcid-extractor

added unit tests

ff09d07

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

21edbda

…mmon into feature/mx-1677-orcid-extractor

update changelog

ebf54ec

vyvytranngoc added 3 commits February 4, 2025 10:38

re-installed dependencies and merge

f69a18f

adjusted unit test for orcid donnector

4da3dd7

Merge branch 'main' of https://github.com/robert-koch-institut/mex-co…

96ab8fb

…mmon into feature/mx-1677-orcid-extractor

cutoffthetop requested changes Feb 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/mx 1677 orcid extractor #361

Feature/mx 1677 orcid extractor #361

vyvytranngoc commented Jan 9, 2025 •

edited

Loading

vyvytranngoc commented Jan 9, 2025

cutoffthetop commented Jan 10, 2025 •

edited

Loading

cutoffthetop commented Jan 10, 2025

cutoffthetop left a comment

cutoffthetop left a comment

vyvytranngoc commented Jan 30, 2025 •

edited by cutoffthetop

Loading

vyvytranngoc commented Jan 30, 2025

cutoffthetop left a comment

cutoffthetop Feb 4, 2025

cutoffthetop Feb 4, 2025

cutoffthetop Feb 4, 2025

cutoffthetop Feb 4, 2025

cutoffthetop Feb 4, 2025

cutoffthetop Feb 4, 2025

	query = OrcidConnector.build_query(filters)
	query = self.build_query(filters)

	orcid_data = OrcidConnector.get_data_by_name(
	orcid_data = OrcidConnector.get().get_data_by_name(

Feature/mx 1677 orcid extractor #361

Are you sure you want to change the base?

Feature/mx 1677 orcid extractor #361

Conversation

vyvytranngoc commented Jan 9, 2025 • edited Loading

PR Context

Added

Changes

Deprecated

Removed

Fixed

Security

vyvytranngoc commented Jan 9, 2025

cutoffthetop commented Jan 10, 2025 • edited Loading

cutoffthetop commented Jan 10, 2025

cutoffthetop left a comment

Choose a reason for hiding this comment

cutoffthetop left a comment

Choose a reason for hiding this comment

vyvytranngoc commented Jan 30, 2025 • edited by cutoffthetop Loading

vyvytranngoc commented Jan 30, 2025

cutoffthetop left a comment

Choose a reason for hiding this comment

cutoffthetop Feb 4, 2025

Choose a reason for hiding this comment

cutoffthetop Feb 4, 2025

Choose a reason for hiding this comment

cutoffthetop Feb 4, 2025

Choose a reason for hiding this comment

cutoffthetop Feb 4, 2025

Choose a reason for hiding this comment

cutoffthetop Feb 4, 2025

Choose a reason for hiding this comment

cutoffthetop Feb 4, 2025

Choose a reason for hiding this comment

vyvytranngoc commented Jan 9, 2025 •

edited

Loading

cutoffthetop commented Jan 10, 2025 •

edited

Loading

vyvytranngoc commented Jan 30, 2025 •

edited by cutoffthetop

Loading