Islandora / controlled_access_terms

Drupal module for subject and agents
GNU General Public License v2.0
7 stars 28 forks source link

Add lcmpt. #80

Closed ruebot closed 2 years ago

ruebot commented 2 years ago

GitHub Issue: n/a

What does this Pull Request do?

Just add lcmpt as a genre authority source.

What's new?

See above.

How should this be tested?

Interested parties

@Islandora/committers

seth-shaw-unlv commented 2 years ago

I presume this addition is fine, but do any of the Metadata Interest Group folks want to chime in about adding it?

Tagging @rosiel @mbolam @kspurgin @rtilla1

kspurgin commented 2 years ago

Seems fine to me.

Only thing I noticed, which may or may not matter:

Looks like the (...I'm forgetting the right term... the part in the URI that identifies the vocabulary. I want to say namespace but I don't think that is right) is performanceMediums, not lcmpt. https://id.loc.gov/authorities/performanceMediums/mp2013015252.html

I have to run to a meeting, so don't have time to dig in further looking for a schema or specs or whatever.

Any reason to code it as that instead?

seth-shaw-unlv commented 2 years ago

Nice catch, @kspurgin. That also reminds me that, technically, the namespace prefix doesn't really matter because it will eventually be mapped to a fully-qualified namespace URI. That said, I neglected to check if this was.

We need to add the namespace prefix to namespace URI map elsewhere, possibly in the controlled_access_terms_rdf_namespaces function, although there are other ways to add it in.

@ruebot, can you make that update.

ruebot commented 2 years ago

@kspurgin @seth-shaw-unlv, sorry I'm confused at what is being asked here.

Are you asking for an updated like this?

      lcgft: 'Library of Congress Genre/Form Terms'
      performanceMediums: 'Library of Congress Medium of Performance Thesaurus

If so, all the others would technically need to be updated too, right? lcgft would become genreForms. I was under the impression that this was just a key/value pair, and it didn't really matter what the key was, which is what I think Seth is saying.

Or is it the latter? If so, I'm not seeing lcgft or anything else of that ilk in controlled_access_terms_rdf_namespaces function. Nor am I seeing anything like that in update_jsonld_included_namespaces, or islandora_defaults_rdf_namespaces.

Apologies if I'm misunderstanding things.

seth-shaw-unlv commented 2 years ago

I see. I didn't realize that LOC doesn't necessarily use their vocabularies' common abbreviations for their namespace URIs.

Looking at this again, I realized that I was conflating the authority sources with typed relation. 🤦‍♂️ Mea cupla! What I said applied to the typed relation whereas the JSON-LD doesn't actually care about the authority source configuration. That is used almost exclusively for display. So, we don't need to add anything.

So, lcmpt v. performanceMediums has no practical implication and this can be merged as-is unless other metadata folks have a strong preference for the "performanceMediums" value.

kspurgin commented 2 years ago

I don't have a strong opinion on it; was just noting it in case it was an issue. Sounds like it is not.

ruebot commented 2 years ago

@seth-shaw-unlv @kspurgin thank you!!