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.

Sign up for GitHub

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

Jump to bottom

NXLog needs fixes to its definition #617

Closed
SkyToGround opened this issue Apr 11, 2018 · 1 comment · Fixed by #618
Closed

NXLog needs fixes to its definition #617

SkyToGround opened this issue Apr 11, 2018 · 1 comment · Fixed by #618
Assignees
@zjttoefs
Milestone
NXDL 2018.5

Comments

@SkyToGround
Copy link

There are (as far as I can tell) two issues with NXLog at the moment:

  1. cue_timestamp_zero should be of type NX_NUMBER as the (optional) unit can not be NX_TIME if the type is an (ISO8601) string.
  2. time should be of type NX_NUMBER as timestamping must allow for integer timestamps (see issue allow NX_NUMBER for arrays with times #593).
@zjttoefs zjttoefs self-assigned this Apr 11, 2018
@zjttoefs zjttoefs added this to the NXDL 2018.4 milestone Apr 11, 2018
zjttoefs added a commit that referenced this issue Apr 11, 2018
@zjttoefs
editorial corrections to NXlog
2aabe16 
fixes #617
make cue_timestamp_zero a number as intended, not a string
allow NX_NUMBER for time, like in NXevent_data
clarify what a missing @start means in cue_timestamp_zero
@zjttoefs zjttoefs mentioned this issue Apr 11, 2018
Merged
@prjemian
Copy link
Contributor

Item 1 actually must have type=NX_CHAR if the type is an (ISO8601) string. Is that what we want?

I agree with 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zjttoefs zjttoefs

Labels
None yet
Projects
None yet
Milestone
NXDL 2018.5
Development

Successfully merging a pull request may close this issue.

editorial corrections to NXlog nexusformat/definitions
3 participants
@SkyToGround @zjttoefs @prjemian