ckan / ckanext-dcat

CKAN ♥ DCAT
163 stars 142 forks source link

Typo in EuropeanDCATAPProfile: publisher_url #92

Closed Faldrian closed 7 years ago

Faldrian commented 7 years ago

I studied the EuropeanDCATAPProfile Implementation in profiles.py and discovered (probably) a typo.

https://github.com/ckan/ckanext-dcat/blob/master/ckanext/dcat/profiles.py#L935

The property name anywhere else is "publisher_uri" and here "publisher_url" is used when writing values to the RDF-Graph. If it really is a typo and should be fixed, I would file a pull request if needed, but I want confirmation first. :)

amercader commented 7 years ago

@Faldrian it is not a typo, publisher_uri and publisher_url are different things. For instance in this RDF document:

https://github.com/ckan/ckanext-dcat/blob/master/examples/dataset.rdf#L65:L72

publisher_uri is http://orgs.vocab.org/some-org and publisher_url is http://some.org. They are stored separately and also used separately to generate a graph.

Faldrian commented 7 years ago

Thanks for the reply :)

Okay ... but the point that didn't add up and made me think this was not intended in the code was the mismatch between the if-block asking for the existence of "publisher_uri" and writing "publisher_url" if it exists. This is confusing and I still think this should be cleaned up or made clear.

amercader commented 7 years ago

The if-block checks the existance of any field that we can use to create a node that reliably identifies the publisher. We can use publisher_uri, publisher_name or the dataset organization for this (but not publisher_url). I think it's pretty clear from the code but please send a PR if you can think of a better way.

Cheers

Faldrian commented 7 years ago

I looked at the "_publisher" method and now I understand ... the uri is the general rdf:about-identifier and the url is some property of the publisher. I thought they meant something similar, it's clear now. Thanks. :)

amercader commented 7 years ago

@Faldrian glad is clear now! Thanks for checking the extension.