Openpmd-api: Warn on unsupported openPMDextensions

Created on 22 Jun 2018  路  5Comments  路  Source: openPMD/openPMD-api

Series::openPMDextension() != 0 may literally break the standard with everyhing we assume in this API w.r.t. metadata, structure and datasets.

Reading/writing any Series with Series::openPMDextension() != 0 may succeed, depending on the used extensions (e.g. if they only _add_ information to the base standard). It is thus not advisable to stop execution when that condition is encountered.
Likewise, reading/writing may fail (e.g. if the extensiton _modifies_ or _removes_ information in the base standard). This will cause a failure at runtime.

We should thus at least notify the user (e.g. with a warning) that the current implementation does not support one or more of the requested/present extensions.

discussion good first issue help wanted internal warning

Most helpful comment

Even if https://github.com/openPMD/openPMD-standard/issues/182 is just a proposal, it's a hint at possible extensions breaking the standard.
I am not opposed to such extensions. Some people might simply not need certain attributes or define them a different way. Which is fine, as long as you clearly state that in the extension specification.

Sidenote: Having ~such extensions~ extensions in general could result in irresolvable conflicts (between extensions).

I always thought that's implied.

Which is the reassuring way of saying it's undefined.

All 5 comments

Hm, I think we don't state it anywhere, but extensions must not break compatibility with the base standard. I always thought that's implied.

Even if https://github.com/openPMD/openPMD-standard/issues/182 is just a proposal, it's a hint at possible extensions breaking the standard.
I am not opposed to such extensions. Some people might simply not need certain attributes or define them a different way. Which is fine, as long as you clearly state that in the extension specification.

Sidenote: Having ~such extensions~ extensions in general could result in irresolvable conflicts (between extensions).

I always thought that's implied.

Which is the reassuring way of saying it's undefined.

Yes, but I mean seriously: we (should) just say in the standard that changing meaning in extensions is fine, but removing required attributes and structure is not. There is not much in the base standard that is fully required, e.g. namings of records is fully open (besides positions so far, but we could generalize the base standard if that is needed). It's really just directory structure, encoding format and attributes.

I mean I see your point, I just don't thing it helps anyone to allow such extensions (as official). Maybe even take a second look at https://github.com/openPMD/openPMD-standard/pull/183 if we overlooked something that could break base compatibility. The issue you referenced above is intentionally not accepted.

I do not have bias either way and will abstain from giving either side a vote. That's a headache I'll leave for extension proposers will have to deal with.

Your point of providing a proper base that no extenstion could reduce further is very good. One example I always come back to in my mind is storing RGB picture data as a field. You would likely never need a unitDimension for Records as the pixel values are intensities [0; 1] that are not part of SI. In that case, one could think of a _highly theoretical_ extension that removes unitDimension.

changing meaning in extensions is fine, but removing required attributes and structure is not

Anyway, this is absolutely fine with me as well.

Your point of providing a proper base that no extenstion could reduce further is very good.
[...]

changing meaning in extensions is fine, but removing required attributes and structure is not

Anyway, this is absolutely fine with me as well.

Yes, but I still think your exact question needs to be addressed in the standard to make it clear we are doing it that way. Probably in the general part where we introduce extensions.

You would likely never need a unitDimension for Records as the pixel values

oh no, that's a common misunderstanding about unitDimension. It is very important in that case. It will tell the reader if it is really a unitless number (it has no dimension). That's not a glitch.

For example, you can still use unitSI to scale a unitless record component.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

pordyna picture pordyna  路  4Comments

C0nsultant picture C0nsultant  路  6Comments

ilomi picture ilomi  路  3Comments

KseniaBastrakova picture KseniaBastrakova  路  6Comments

ax3l picture ax3l  路  4Comments