Skip to content

Add 'rustc-serialize' and 'serde-serialize' features.#134

Closed
qrlpx wants to merge 1 commit intodimforge:masterfrom
qrlpx:master
Closed

Add 'rustc-serialize' and 'serde-serialize' features.#134
qrlpx wants to merge 1 commit intodimforge:masterfrom
qrlpx:master

Conversation

@qrlpx
Copy link
Copy Markdown

@qrlpx qrlpx commented Jun 13, 2015

This allows users to choose rustc-serialize or serde (or both or none) for serializiation.
'rustc-serialize' is a default feature, serde is activated through the 'serde-serialize' feature.
If there's any interest in it, I'll eventually do the same for ncollide and nphysics.

@sebcrozet
Copy link
Copy Markdown
Member

I'm a bit concerned about this feature-based approach.

If two libraries end up using the same version of nalgebra but with different serialization methods (i-e, different features), they won't be able to cooperate smoothly (because cargo will identify both nalgebra libraries as different). And I don't feel very comfortable introducing this kind of potential incompatibility for a low level mathematical library like that.

@erickt
Copy link
Copy Markdown

erickt commented Aug 20, 2015

serde serializers and deserializers can live side by side with rustc-serialize encoders and decoders, so I don't think these need to be optional. There is some minor trickiness required to #[derive(Serialize, Deserialize)] in stable rust, but I'd be happy to help out with that if you're interested.

@bluss
Copy link
Copy Markdown

bluss commented Dec 20, 2015

@sebcrozet That's not my impression. Cargo will union the feature flags, and it will be just one compilation of nalgebra. (And features need to be additive for this reason).

I'm not actually sure the docs cover this well enough. http://doc.crates.io/manifest.html#the-[features]-section

@bluss
Copy link
Copy Markdown

bluss commented Dec 20, 2015

@erickt They should be optional though, since it slows down the compile too much otherwise?

@Boscop
Copy link
Copy Markdown

Boscop commented Nov 2, 2016

I'm very interested in serialization support as well!

@robo-corg
Copy link
Copy Markdown

Now that rust 1.15 is out and rust_derive works on stable this seems like a great idea.

@vadixidav
Copy link
Copy Markdown
Contributor

I am also looking for this right now, particularly now that serde_derive is supported on stable. This PR needs to be modified to use it though.

@robo-corg
Copy link
Copy Markdown

I have an updated version of the pull request in a branch. Tests seem to pass but I haven't tried using the traits yet: https://github.com/robo-hamburger/nalgebra

@sebcrozet
Copy link
Copy Markdown
Member

Thanks for your contributions and sorry I've forgotten about this issue. As part of the massive changes that has been done for #218 (related issue: #207 ), I added the support for serde behind the serde-serialize feature. I've removed rustc-serialize though as it seems to be much less practical.

@sebcrozet sebcrozet closed this Feb 15, 2017
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.

7 participants