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.

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

Fix key-value writing: no space allowed after ':=' identifier #69

Merged
merged 1 commit into from
Sep 24, 2018

Conversation

ihnorton
Copy link
Contributor

This was changed in #57

but per the NRRD documentation: http://teem.sourceforge.net/nrrd/format.html#general.2

"""
Each of the ":=" lines specifies a key/value pair in the nrrd. These can appear in NRRD0002 (and higher version) files, but not NRRD0001 files. The key and value strings are delimited by the first ":=" to appear on the line: any spaces before or after ":=" are assumed part of the key or value, respectively.
"""

per the NRRD documentation: http://teem.sourceforge.net/nrrd/format.html#general.2

"""
Each of the "<key>:=<value>" lines specifies a key/value pair in the nrrd. These can appear in NRRD0002 (and higher version) files, but not NRRD0001 files. The key and value strings are delimited by the first ":=" to appear on the line: any spaces before or after ":=" are assumed part of the key or value, respectively.
"""
@codecov-io
Copy link

codecov-io commented Sep 21, 2018

Codecov Report

Merging #69 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #69   +/-   ##
=======================================
  Coverage   87.39%   87.39%           
=======================================
  Files           6        6           
  Lines         357      357           
  Branches      114      114           
=======================================
  Hits          312      312           
  Misses         22       22           
  Partials       23       23
Impacted Files Coverage Δ
nrrd/writer.py 87.17% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21366d2...1500252. Read the comment docs.

@addisonElliott
Copy link
Collaborator

Good catch! 👍

Do you think we should not remove whitespace from the key/value pairs when reading NRRD files? Personally, I lean towards no because I can't think of any useful reason of having whitespace in the value.

And, when I merge this in (I'll try to tonight), I'll probably wait to release the next version unless you really need these changes now. Hopefully a few more PRs come in soon and I can make a larger release.

@ihnorton
Copy link
Contributor Author

Do you think we should not remove whitespace from the key/value pairs when reading NRRD files? Personally, I lean towards no because I can't think of any useful reason of having whitespace in the value.

Agree, try to stick to spec when possible. I think some readers de-facto allow whitespace after the := (e.g. if using >> operator to read whitespace-separated values in C++) but some software we use does not.

@tashrifbillah
Copy link
Contributor

Hi @addisonElliott , can we get a release incorporating @ihnorton 's latest changes?
The release 0.3.4 does not have Isaiah's latest commit.

@addisonElliott
Copy link
Collaborator

Sure, I'll release a new version tonight.

@addisonElliott
Copy link
Collaborator

v0.3.5 Released! 🎉

@tashrifbillah
Copy link
Contributor

Thanks @addisonElliott . We have also switched to the release version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants