openMetadataInitiative / openMINDS_Python

Python package for the openMINDS metadata models. The package contains all openMINDS schemas as Python classes in addition to schema base classes and utility methods.
MIT License
1 stars 2 forks source link

Allow non http IRIs #37

Closed apdavison closed 8 months ago

apdavison commented 8 months ago

Instead, added a validation check that will flag file:/// IRIs

lzehl commented 8 months ago

@apdavison this would be a workaround to a missing / incomplete schema definition correct? I would assume a cleaner solution would be an update in the actual schemas.

Option 1)

Option 2)

apdavison commented 8 months ago

LocalFile was intended more for files that no longer exist.

The use case here is more for people preparing openMINDS collections locally before they've uploaded their data to a public repository. The idea is that they would use "file://" URLs when preparing the collection, then replace these with the correct "https://" URLs once the files are uploaded.

lzehl commented 8 months ago

@apdavison thanks for the clarification. LocalFile was in my eye exactly used for files stored locally. So there was a misunderstanding when we created that. However the use case you describe is something which we should support on the schema level (not only as workaround within the python package) no?

apdavison commented 8 months ago

I don't think there was really an understanding about LocalFile. If a file is stored locally, it doesn't really exist from the perspective of linked data, so LocalFile is for recording that there was once a file at this path, which may or may not still exist.

Doesn't the schema support file:/// IRIs? I don't see anything in the schema definition that precludes it? I didn't see this as a Python-specific workaround, rather this change makes the Python package more schema-compliant.

lzehl commented 8 months ago

@apdavison you are right I think. The assumption in the schema is currently that an IRIs follow the string format set by JSON-Schema: https://json-schema.org/understanding-json-schema/reference/string#resource-identifiers (which should accept file://). Sorry for the confusion...

lzehl commented 8 months ago

@apdavison do you want to update the regular expression for IRIs to fit RFC3987? if not, the PR looks good to merge from my side

apdavison commented 8 months ago

update the regular expression for IRIs to fit RFC3987

done. I've used an external package, but it seems to be small and well maintained, so I think it's reasonable to add as a dependency.

Please go ahead and merge if you're happy with it.