Skip to content
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

Added new Class OldResource and new property wasPartOf #38

Merged
merged 5 commits into from
Feb 19, 2025

Conversation

bellerophons-pegasus
Copy link
Member

New class and property for issue #37

@bellerophons-pegasus
Copy link
Member Author

Stiff a few things to fix

@bellerophons-pegasus bellerophons-pegasus marked this pull request as draft February 3, 2025 11:21
@zozlak
Copy link
Member

zozlak commented Feb 3, 2025

@bellerophons-pegasus acdh:Resource and acdh:OldResource require a common parent class so the acdh:hasPixelWidth and acdh:hesPixelHeight can denote it as its domain - see the issues reported by the automatic check

This common parent will most likely be just yet another helper class. By the way all shared cardinality constraints should be defined on it and only acdh:isPartOf/acdh:wasPartOf cardinality should be defined on the acdh:Resource/acdh:OldResource level.

@bellerophons-pegasus
Copy link
Member Author

Fixed issues. Ready for review

@bellerophons-pegasus bellerophons-pegasus marked this pull request as ready for review February 4, 2025 10:34
@zozlak
Copy link
Member

zozlak commented Feb 10, 2025

So the way we proceed now goes as follows:

  • I'm adjusting the arche-lib-ingest and/or arche-ingest libraries so they are capable of generating the acdh:oldResource and I'm testing that the automatic single file versioning on ingestion works.
  • Then (when we have a live example) we ask Norbert to check if it doesn't break the GUI. Once we confirm/assure it doesn't, we merge and publish.

@sstuhec
Copy link
Contributor

sstuhec commented Feb 10, 2025

I just have one comment/question:

Currently the acdh:wasPartOf has range "oldResource" and domain "CollectionOrPlaceOrPublication".

I assume this is because acdh:isPartOf has this domain, however, acdh:wasPartOf is specifically for an oldResource and a Resource (new or old) cannot have a range Place nor Publication. Place and Publication also cannot have "versions" (acdh:hasVersion) according to our ontology (we could discuss, if Publications could once have it, but it is currently not the case).

So, should the domain not rather be "TopCollection+Collection" (or whatever is the helper for this)?

@bellerophons-pegasus
Copy link
Member Author

Good point.

To recap:
For acdh:isPartOf the domain and range were intentionally chosen as they are now to allow for such cases where a place is part of a larger place (e.g. a city within a country), or, for publications, a chapter within an edited volume.

So technically, a Resource that is part of a Place would not violate our ontology. This is one of quite a few cases were our ontology is still 'fuzzy' and 'flexible' and not too strict.
To avoid such a case, we have to rely on the curators. If we would like to also enforce this on the level of the ontology we would need two additional properties (one for places and another for publications).

But yes, the domain for acdh:wasPartOf can be narrower

@zozlak
Copy link
Member

zozlak commented Feb 13, 2025

I prepared and released arche-lib-ingest v5 and arche-ingest v1.5 which should do the metadata management in a way which is in line with the schema changes proposed by this pull request.

The ontology from this request is ingested on the dev instance.

@csae8092 could you please give it a test run on the dev instance?

@zozlak
Copy link
Member

zozlak commented Feb 13, 2025

@bellerophons-pegasus do you have something against integrating the #40 into this pull request?

@zozlak
Copy link
Member

zozlak commented Feb 18, 2025

If anyone has any objections, please speak now!
Otherwise tomorrow I'm merging the pull and making a release.

@zozlak zozlak merged commit 6cf686b into master Feb 19, 2025
2 checks passed
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.

4 participants