-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/mx 1677 orcid extractor #361
base: main
Are you sure you want to change the base?
Changes from all commits
3e392cf
20d9727
82a859d
57cd6ac
06040ad
fa27e0e
136c725
67ba02b
90a2b65
9f7f13e
b39c714
d86029b
19fa4a4
93d50c3
781ccaa
74c0346
bef1ee6
3ca8a6d
ff09d07
21edbda
ebf54ec
f69a18f
4da3dd7
96ab8fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,89 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import Any | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
from mex.common.connector.http import HTTPConnector | ||||||||||||||||||||||||||||||||||||||||||||||||||||
from mex.common.exceptions import EmptySearchResultError, FoundMoreThanOneError | ||||||||||||||||||||||||||||||||||||||||||||||||||||
from mex.common.settings import BaseSettings | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
class OrcidConnector(HTTPConnector): | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Connector class for querying Orcid records.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
def _set_url(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Set url of the host.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
settings = BaseSettings.get() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
self.url = str(settings.orcid_api_url) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
def _check_availability(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Send a GET request to verify the host is available.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
url = f"{self.url.rstrip('/')}/search" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
response = self._send_request("HEAD", url=url, params={}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
response.raise_for_status() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
def check_orcid_id_exists(self, orcid_id: str) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Search for an ORCID person by ORCID ID.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
query_dict = {"orcid": orcid_id} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
response = self.fetch(query_dict) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
return bool(response.get("num-found", 0)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||
def build_query(filters: dict[str, Any]) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Construct the ORCID API query string.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
return " AND ".join([f"{key}:{value}" for key, value in filters.items()]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
def fetch(self, filters: dict[str, Any]) -> dict[str, Any]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Perform a search query against the ORCID API.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
query = OrcidConnector.build_query(filters) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
return self.request(method="GET", endpoint="search", params={"q": query}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
@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)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+38
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why does this have to be static?
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||
def get_data_by_name( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, why make this static? instead of |
||||||||||||||||||||||||||||||||||||||||||||||||||||
given_names: str = "*", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
family_name: str = "*", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
**filters: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
) -> dict[str, Any]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Get ORCID record of a single person for the given filters. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
Args: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
self: Connector. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
given_names: Given name of a person, defaults to non-null | ||||||||||||||||||||||||||||||||||||||||||||||||||||
family_name: Surname of a person, defaults to non-null | ||||||||||||||||||||||||||||||||||||||||||||||||||||
**filters: Key-value pairs representing ORCID search filters. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
Raises: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
EmptySearchResultError | ||||||||||||||||||||||||||||||||||||||||||||||||||||
FoundMoreThanOneError | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
Returns: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Orcid data of the single matching person by name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
if given_names: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
filters["given-names"] = given_names | ||||||||||||||||||||||||||||||||||||||||||||||||||||
if family_name: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
filters["family-name"] = family_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||
orcidapi = OrcidConnector.get() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
search_response = orcidapi.fetch(filters=filters) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
num_found = search_response.get("num-found", 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
if num_found == 0: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
msg = f"Cannot find orcid person for filters {filters}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
raise EmptySearchResultError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
if num_found > 1: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
msg = f"Found multiple orcid persons for filters {filters}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
raise FoundMoreThanOneError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
orcid_id = search_response["result"][0]["orcid-identifier"]["path"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
return OrcidConnector.get_data_by_id(orcid_id) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,39 @@ | ||||||
from mex.common.orcid.connector import OrcidConnector | ||||||
from mex.common.orcid.models.person import OrcidRecord | ||||||
from mex.common.orcid.transform import map_orcid_data_to_orcid_record | ||||||
|
||||||
|
||||||
def get_orcid_record_by_name( | ||||||
given_names: str = "*", family_name: str = "*" | ||||||
) -> OrcidRecord: | ||||||
"""Returns Orcidrecord of a single person for the given filters. | ||||||
|
||||||
Args: | ||||||
given_names: Given name of a person, defaults to non-null | ||||||
family_name: Surname of a person, defaults to non-null | ||||||
**filters: Key-value pairs representing ORCID search filters. | ||||||
|
||||||
Raises: | ||||||
EmptySearchResultError | ||||||
FoundMoreThanOneError | ||||||
|
||||||
Returns: | ||||||
Orcidrecord of the matching person by name. | ||||||
""" | ||||||
orcid_data = OrcidConnector.get_data_by_name( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when you make get_data_by_name non-static, this would become:
Suggested change
|
||||||
given_names=given_names, family_name=family_name | ||||||
) | ||||||
return map_orcid_data_to_orcid_record(orcid_data) | ||||||
|
||||||
|
||||||
def get_orcid_record_by_id(orcid_id: str) -> OrcidRecord: | ||||||
"""Returns Orcidrecord by UNIQUE ORCID ID. | ||||||
|
||||||
Args: | ||||||
orcid_id: Unique identifier in ORCID system. | ||||||
|
||||||
Returns: | ||||||
Orcidrecord of the matching id. | ||||||
""" | ||||||
orcid_data = OrcidConnector.get_data_by_id(orcid_id=orcid_id) | ||||||
return map_orcid_data_to_orcid_record(orcid_data) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
from pydantic import Field | ||
|
||
from mex.common.models import BaseModel | ||
|
||
|
||
class OrcidIdentifier(BaseModel): | ||
"""Model class for OrcidID.""" | ||
|
||
path: str | ||
uri: str | ||
|
||
|
||
class OrcidEmail(BaseModel): | ||
"""Model class for Orcid email.""" | ||
|
||
email: list[str] | ||
|
||
|
||
class OrcidEmails(BaseModel): | ||
"""Model class for Orcid emails.""" | ||
|
||
email: list[OrcidEmail] | ||
|
||
|
||
class OrcidFamilyName(BaseModel): | ||
"""Model class for orcid family names.""" | ||
|
||
value: str | ||
|
||
|
||
class OrcidGivenNames(BaseModel): | ||
"""Model class for Orcid given names.""" | ||
|
||
value: str | ||
|
||
|
||
class OrcidName(BaseModel): | ||
"""Model class for Orcid name.""" | ||
|
||
family_name: OrcidFamilyName = Field(alias="family-name") | ||
given_names: OrcidGivenNames = Field(alias="given-names") | ||
visibility: str | ||
|
||
|
||
class OrcidPerson(BaseModel): | ||
"""Model class for Orcid person.""" | ||
|
||
emails: OrcidEmails | ||
name: OrcidName | ||
|
||
|
||
class OrcidRecord(BaseModel): | ||
"""Model class for Orcid record.""" | ||
|
||
orcid_identifier: OrcidIdentifier = Field(alias="orcid-identifier") | ||
person: OrcidPerson |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
from typing import Any | ||
|
||
from mex.common.models import ( | ||
ExtractedPerson, | ||
) | ||
from mex.common.orcid.models.person import OrcidRecord | ||
from mex.common.primary_source.helpers import get_all_extracted_primary_sources | ||
|
||
|
||
def map_orcid_data_to_orcid_record(orcid_data: dict[str, Any]) -> OrcidRecord: | ||
"""Wraps orcid data into an OrcidRecord.""" | ||
return OrcidRecord.model_validate(orcid_data) | ||
|
||
|
||
def transform_orcid_person_to_mex_person( | ||
orcid_record: OrcidRecord, | ||
) -> ExtractedPerson: | ||
"""Transforms a single ORCID person to an ExtractedPerson. | ||
|
||
Args: | ||
orcid_record: OrcidRecord object of a person. | ||
|
||
Returns: | ||
ExtractedPerson. | ||
""" | ||
primary_source = get_all_extracted_primary_sources()["orcid"] | ||
had_primary_source = primary_source.stableTargetId | ||
|
||
id_in_primary_source = orcid_record.orcid_identifier.path | ||
email = orcid_record.person.emails.email[0].email | ||
if orcid_record.person.name.visibility == "public": | ||
given_names = orcid_record.person.name.given_names.value | ||
family_name = orcid_record.person.name.family_name.value | ||
else: | ||
given_names = None | ||
family_name = None | ||
orcid_id = orcid_record.orcid_identifier.uri | ||
|
||
return ExtractedPerson( | ||
identifierInPrimarySource=id_in_primary_source, | ||
hadPrimarySource=had_primary_source, | ||
givenName=given_names, | ||
familyName=family_name, | ||
orcidId=orcid_id, | ||
email=email, | ||
) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,13 +12,15 @@ | |||||||||||||||||
from typing import Any, cast | ||||||||||||||||||
from unittest.mock import MagicMock, Mock | ||||||||||||||||||
|
||||||||||||||||||
import pytest | ||||||||||||||||||
import requests | ||||||||||||||||||
from langdetect import DetectorFactory | ||||||||||||||||||
from pydantic import AnyUrl | ||||||||||||||||||
from requests import Response | ||||||||||||||||||
from requests import HTTPError, Response | ||||||||||||||||||
|
||||||||||||||||||
from mex.common.connector import CONNECTOR_STORE | ||||||||||||||||||
from mex.common.models import ExtractedPrimarySource | ||||||||||||||||||
from mex.common.orcid.connector import OrcidConnector | ||||||||||||||||||
from mex.common.primary_source.helpers import get_all_extracted_primary_sources | ||||||||||||||||||
from mex.common.settings import SETTINGS_STORE, BaseSettings | ||||||||||||||||||
from mex.common.wikidata.connector import ( | ||||||||||||||||||
|
@@ -191,3 +193,67 @@ def get_wikidata_item_details_by_id( | |||||||||||||||||
"get_wikidata_item_details_by_id", | ||||||||||||||||||
get_wikidata_item_details_by_id, | ||||||||||||||||||
) | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
@pytest.fixture | ||||||||||||||||||
def orcid_person_raw() -> dict[str, Any]: | ||||||||||||||||||
"""Return a raw orcid person.""" | ||||||||||||||||||
with open(Path(__file__).parent / "test_data" / "orcid_person_raw.json") as fh: | ||||||||||||||||||
return cast(dict[str, Any], json.load(fh)) | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
@pytest.fixture | ||||||||||||||||||
def orcid_multiple_matches() -> dict[str, Any]: | ||||||||||||||||||
"""Return a raw orcid person.""" | ||||||||||||||||||
with open( | ||||||||||||||||||
Path(__file__).parent / "test_data" / "orcid_multiple_matches.json" | ||||||||||||||||||
) as fh: | ||||||||||||||||||
return cast(dict[str, Any], json.load(fh)) | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
@pytest.fixture | ||||||||||||||||||
def mocked_orcid( | ||||||||||||||||||
monkeypatch: pytest.MonkeyPatch, | ||||||||||||||||||
orcid_person_raw: dict[str, Any], | ||||||||||||||||||
orcid_multiple_matches: dict[str, Any], | ||||||||||||||||||
) -> None: | ||||||||||||||||||
"""Mock orcid connector.""" | ||||||||||||||||||
response_query = Mock(spec=Response, status_code=200) | ||||||||||||||||||
|
||||||||||||||||||
session = MagicMock(spec=requests.Session) | ||||||||||||||||||
session.get = MagicMock(side_effect=[response_query]) | ||||||||||||||||||
|
||||||||||||||||||
def mocked_init(self: OrcidConnector) -> None: | ||||||||||||||||||
self.session = session | ||||||||||||||||||
|
||||||||||||||||||
monkeypatch.setattr(OrcidConnector, "__init__", mocked_init) | ||||||||||||||||||
|
||||||||||||||||||
def check_orcid_id_exists(_self: OrcidConnector, _orcid_id: str) -> bool: | ||||||||||||||||||
return _orcid_id == "0009-0004-3041-5706" | ||||||||||||||||||
|
||||||||||||||||||
monkeypatch.setattr(OrcidConnector, "check_orcid_id_exists", check_orcid_id_exists) | ||||||||||||||||||
|
||||||||||||||||||
def fetch(_self: OrcidConnector, filters: dict[str, Any]) -> dict[str, Any]: | ||||||||||||||||||
if filters.get("given-names") == "John": | ||||||||||||||||||
return {"num-found": 1, "result": [orcid_person_raw]} | ||||||||||||||||||
if filters.get("given-names") == "Multiple": | ||||||||||||||||||
return orcid_multiple_matches | ||||||||||||||||||
return {"result": None, "num-found": 0} | ||||||||||||||||||
|
||||||||||||||||||
monkeypatch.setattr(OrcidConnector, "fetch", fetch) | ||||||||||||||||||
|
||||||||||||||||||
def get_data_by_id(orcid_id: str) -> dict[str, Any]: | ||||||||||||||||||
if orcid_id == "0009-0004-3041-5706": | ||||||||||||||||||
return orcid_person_raw | ||||||||||||||||||
msg = "404 Not Found" | ||||||||||||||||||
raise HTTPError(msg) | ||||||||||||||||||
|
||||||||||||||||||
monkeypatch.setattr(OrcidConnector, "get_data_by_id", staticmethod(get_data_by_id)) | ||||||||||||||||||
|
||||||||||||||||||
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)) | ||||||||||||||||||
Comment on lines
+252
to
+259
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
you don't need to mock this method. since it does not do any network-io, it is "unit-test-safe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method is unused in mex-common, do you need it for mex-backend?