search

binary-array-ld / net.binary_array_ld.bald

Java library for parsing, validating and managing binary array linked data files, e.g. HDF, netCDF.
Apache License 2.0
0 stars 3 forks source link

Feature/11.1 var reference single #26

Closed simonoakesepimorphics closed 3 years ago

simonoakesepimorphics commented 3 years ago

Support attribute variable references (single valued only).

Updated with merge from #27 .

marqh commented 3 years ago

Hello @simonoakesepimorphics @CMayNorris

given the volume of change on this PR, I think it would lend itself to an online VOIP discussion between the three of us.

I'd like to identify key points and actions, and document them on the ticket, to enable a targeted review to follow aiming to adopt this code.

please may we communicate via email to schedule a suitable target?

thank you mark

simonoakesepimorphics commented 3 years ago

@marqh @CMayNorris This PR is ready for review now.

simonoakesepimorphics commented 3 years ago

this PR looks great. my only concern is the referenced places where bald:Subject is used, but I think this has been deprecated and does not appear in the vocabulary https://www.opengis.net/def/binary-array-ld

@simonoakesepimorphics please confirm if this can be removed / replaced?

@marqh The bald:Subject usage is based on the spec: http://docs.opengeospatial.org/DRAFTS/19-002.html#_requirement_e_1 . Is there a different type I should use instead?

marqh commented 3 years ago

@marqh The bald:Subject usage is based on the spec: http://docs.opengeospatial.org/DRAFTS/19-002.html#_requirement_e_1 . Is there a different type I should use instead?

There's an open issue on tis on the spec: https://github.com/opengeospatial/netcdf-ld/issues/56

The change to remove bald:Subject was only merged this morning (sorry) and so the builder for the editors draft hasn't run yet. https://github.com/opengeospatial/netcdf-ld/commit/09356c5d9b7b1a31e4e9952aed69121a83d3020f

The answer here is bald:Subject => bald:Resource

thank you for your patience with this confusion

simonoakesepimorphics commented 3 years ago

Sorry, I forgot all about the issue I raised! I'll make the change as soon as I can.

marqh commented 3 years ago

many thanks @simonoakesepimorphics