image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.96k stars 618 forks source link

Added serialization-deserialization and FITS output. #2212

Closed sunipkm closed 5 months ago

sunipkm commented 6 months ago

I license past and future contributions under the dual MIT/Apache-2.0 license, allowing licensees to choose either at their option.

Here is a summary of changes:

fintelia commented 6 months ago

Thanks for your interest in image-rs!

Changes of this scale need to be discussed before we can get to the stage of reviewing a specific PR.

We also strongly prefer breaking up changes as much as possible because it makes them faster to review and reduces the amount of back-and-forth interaction to get any individual change merged. Each of your bullet point summary items likely warrant separate discussion issue and (if we decided the feature was something that belonged in this crate) its own PR.

sunipkm commented 6 months ago

That’s fair, glad to know you are interested. I will break this up into three and submit tomorrow:

  1. The “metadata”.
  2. Serialization-deserialization.
  3. FITS IO.
fintelia commented 6 months ago

Before working on PRs could you create issues to discuss each of those? They all seem useful but I'm not yet sold that they are all things we want to add to this specific crate

sunipkm commented 6 months ago

Through our discussion, I see how metadata is challenging. Storing FITS images also appear to be outside the scope of the image crate, because of the niche use case, and more importantly, the C dependency. However, I think serde is important, and doable, and we should discuss that.