-
Notifications
You must be signed in to change notification settings - Fork 47
Hotfix/umontreal #516
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
Hotfix/umontreal #516
Conversation
add test for one harvester
add invoke clear to remove provider information from elasticsearch
if not languages: | ||
languages = [] | ||
|
||
return languages |
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.
In the original schema, each item in the languages array is transformed to a ISO 639-3 code, for example, 'English' becomes 'eng' and 'Russian' becomes 'rus'. I checked the source and it seems that they were not correctly coded (for example it uses 'en' instead of 'eng'). There is a function get_code in helpers.py that can do that for you. Can you implement it in your function to return the correct code?
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.
Below is the description in the schema:
- "description": "The primary languages in which the content of the resource is presented. Values used for this element MUST conform to ISO 639\u20133. This offers two and three letter tags e.g. "en" or "eng" for English and "en-GB" for English used in the UK."
You can find all the tags here: https://www.loc.gov/standards/iso639-2/php/code_list.php
It is my understanding that the tags are correct - en maps to English and fr maps to French
Can you confirm that this still needs to be done?
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.
@jeffreyliu3230 This is ready to merge, right?
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.
Oh good find! Yes this should be ready to merge.
removed date from properties list. Also updated language to be read in correctly according to schema.