Skip to content

Conversation

@jrcastro2
Copy link
Contributor

No description provided.

@jrcastro2 jrcastro2 force-pushed the codex/implement-field-mappings-for-transform_entry-function branch from dd20fcc to 8882b80 Compare June 17, 2025 14:57
if degree_type:
result["type"] = degree_type
if institutions:
uni = institutions[0].get("name")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we okay with this? any ohter ideas?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I would fail if there are more values than 1


pub_infos = self.inspire_metadata.get("publication_info", [])
if pub_infos:
journal_cf = self._transform_journal(pub_infos[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there might be multple pub_infos ... is it okay to take the first one?

Comment on lines +144 to +145
if lang:
trans_title["lang"] = lang
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be a bit more readable if you assign all values, even if they default to None and clean the empty keys at the end, we will avoid nested if statements

Comment on lines +138 to +151
for translation in translations:
lang = translation.get("language")
title = translation.get("title")
subtitle = translation.get("subtitle")
if title:
trans_title = {"title": title, "type": {"id": "translated-title"}}
if lang:
trans_title["lang"] = lang
rdm_additional_titles.append(trans_title)
if subtitle:
sub = {"title": subtitle, "type": {"id": "subtitle"}}
if lang:
sub["lang"] = lang
rdm_additional_titles.append(sub)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for translation in translations:
lang = translation.get("language")
title = translation.get("title")
subtitle = translation.get("subtitle")
if title:
trans_title = {"title": title, "type": {"id": "translated-title"}}
if lang:
trans_title["lang"] = lang
rdm_additional_titles.append(trans_title)
if subtitle:
sub = {"title": subtitle, "type": {"id": "subtitle"}}
if lang:
sub["lang"] = lang
rdm_additional_titles.append(sub)
for translation in translations:
lang = translation.get("language")
title = translation.get("title")
subtitle = translation.get("subtitle")
type = None
if title:
type = "translated-title"
elif subtitle:
type = "subtitle"
else:
raise
additional_title = {"title": title or subtitle, "type": {"id": "translated-title"}, "lang": lang}
... remove none keys....
rdm_additional_titles.append(additional title)

not sure if it is more readable this way, so it is not a strong opinion,

if not value:
continue

if schema in ["PACS", "CERN LIBRARY"]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets maybe leave a comment that it was agreed to drop these schemes.
Did you see maybe UDC schema in any of the records?

if journal_record := info.get("journal_record"):
related_identifiers.append(
{
"identifier": journal_record,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this info about journal should also go to journal:journal custom field

related_identifiers.append(
{
"identifier": parent_rep,
"scheme": "cdsref",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was

Suggested change
"scheme": "cdsref",
"scheme": "cds_ref",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants