mhe / pynrrd

Simple pure-python module for reading and writing nrrd files.
https://pynrrd.readthedocs.io/
MIT License
116 stars 51 forks source link

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

Closed ihnorton closed 6 years ago

ihnorton commented 6 years ago

This was changed in https://github.com/mhe/pynrrd/pull/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. """

codecov-io commented 6 years ago

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%> (ø) :arrow_up:

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Sure, I'll release a new version tonight.

addisonElliott commented 6 years ago

v0.3.5 Released! 🎉

tashrifbillah commented 6 years ago

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