Enet4 / nifti-rs

Rust implementation of the NIfTI-1 format
Apache License 2.0
42 stars 12 forks source link

Fix Big Endian writing #39

Closed Enet4 closed 5 years ago

Enet4 commented 5 years ago

Also introducing the longest turbo fish I ever made. :neutral_face: This was a consequence of distinguishing the array's element type A from the array's "atomic element" type B. In RGB, A = [u8; 3] and B = u8, whereas so far A = B for other cases. With this extra type parameter, we can use the same functions for both.

Resolves #34.

nilgoyette commented 5 years ago

Sounds good. I don't disagree to merge :)

It's unrelated, but is there a reason why you use really specific versions like num-derive = "0.2.0" instead of a more practical num-derive = "0.2"? 4 of the 12 dependencies are outdated but only safe-transmute is outdated for "real".

Enet4 commented 5 years ago

Thank you for checking this out, I will merge it shortly.

It's unrelated, but is there a reason why you use really specific versions like num-derive = "0.2.0" instead of a more practical num-derive = "0.2"?

Note that num-derive = "0.2.0" does not mean to specificly install version 0.2.0. The default dependency version requirement is the caret requirement (Cargo guide), so it will accept any version without a potential breaking version bump: >= 0.2.0 && < 0.3.0. For 0-major versions, this is practically equivalent to leaving "0.2", except that we can use the same format to specify a minimum patch version component (such as 0.2.3). In more stable crates, the caret requirement would allow both minor and patch version bumps (e.g. the constraint 1.2.0 is honoured by version 1.3.0 and 1.2.1). As such, the default version requirement is a very reasonable constraint.

4 of the 12 dependencies are outdated but only safe-transmute is outdated for "real".

A more accurate check for outdated dependencies in a library is the one made by deps.rs: if a cargo update does not push every dependency in the version lock to their latest versions, then it's outdated. With byteordered updated, it will show only 1 outdated dependency.

I haven't updated safe-transmute yet because it's currently under a heavy re-design phase to favour safety while trying to make it usable.

nilgoyette commented 5 years ago

I verified with cargo.lock and you're right! I have been misled by crates extension of VSCode. It shows almost all dependencies as not-the-latest. Thank you for the explanation.