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

Add approx-0_5 feature for approx 0.5 support #1025

Merged
merged 2 commits into from
Nov 18, 2021

Conversation

jturner314
Copy link
Member

approx 0.5 updates its num-complex dependency to 0.4, which is the same version of num-complex which ndarray depends on. So, it's very useful for ndarray to support approx 0.5.

See also #1024, which attempts to support approx 0.5 by changing the version range to ">=0.4, <0.6". Unfortunately, that doesn't work.

`approx` 0.5 updates its `num-complex` dependency to 0.4, which is the
same version of `num-complex` which `ndarray` depends on. So, it's
very useful for `ndarray` to support `approx` 0.5.
@bluss
Copy link
Member

bluss commented Jun 5, 2021

I think a feature name like approx-05 or approx-0_5 would be preferable. Is the "p" thing used anywhere?

@bluss bluss added this to the 0.15.4 milestone Jun 5, 2021
@jturner314
Copy link
Member Author

I think a feature name like approx-05 or approx-0_5 would be preferable. Is the "p" thing used anywhere?

Okay, I've changed it to approx-0_5.

I guess the "p" thing is unique to me. When naming files/directories for the results of scientific code, I often have one more more parameters (which may not be integers) in the name. I generally avoid dots in filenames (except as the file extension delimiter), I use _ as a word delimiter, and I use - as a minus sign for negative values, so the convention I've adopted is to use p to represent the decimal point. For example, I might name a directory containing results 2021-06-05_83ae587_some_method_name_alpha0p5_beta-7p3.

@bluss bluss changed the title Add approx-0p5 feature for approx 0.5 support Add approx-0_5 feature for approx 0.5 support Jun 6, 2021
@maoe
Copy link

maoe commented Jul 19, 2021

Could anyone elaborate on why the dependency on approx 0.4 has to be kept rather than just updating it to 0.5? It wasn't clear to me either in this PR or in #1024.

@jturner314
Copy link
Member Author

Changing the version of the approx dependency from 0.4 to 0.5 is a breaking change, since ndarray uses traits from approx in the public API (when the approx feature is enabled). It's preferable to minimize the number of breaking releases, and there isn't a strong motivation at this time for a new breaking release of ndarray.

@maoe
Copy link

maoe commented Jul 19, 2021

Do you mean a breaking change in terms of the version requirement? API-wise there is no breaking change between v0.4.0 vs. v0.5.0.

@jturner314
Copy link
Member Author

Yes, the version numbers signal a breaking change. See Cargo's documentation for more information. Let's say a user had the following code:

# Cargo.toml

[package]
name = "foo"
version = "0.1.0"
edition = "2018"

[dependencies]
approx = "0.4.0"
ndarray = { version = "0.15.3", features = ["approx"] }
// src/main.rs

use approx::{assert_abs_diff_eq, AbsDiffEq};
use ndarray::{array, Array1};

#[derive(Debug, PartialEq)]
struct Wrapper(f64);

impl AbsDiffEq<Wrapper> for Wrapper {
    type Epsilon = f64;

    fn default_epsilon() -> f64 {
        f64::default_epsilon()
    }

    fn abs_diff_eq(&self, other: &Wrapper, epsilon: f64) -> bool {
        self.0.abs_diff_eq(&other.0, epsilon)
    }
}

fn main() {
    let a: Array1<Wrapper> = array![Wrapper(0.), Wrapper(1.)];
    let b: Array1<Wrapper> = array![Wrapper(0.), Wrapper(1.)];
    assert_abs_diff_eq!(a, b);
}

Changing ndarray's dependency on approx from 0.4 to 0.5 would break this code. (It would no longer compile, since approx 0.5's AbsDiffEq would not be implemented for Array1<Wrapper>, since ArrayBase's AbsDiffEq implementation has an A: AbsDiffEq<B> bound, and Wrapper would still implement only approx 0.4's AbsDiffEq, not approx 0.5's AbsDiffEq.)

In other words, we can change change ndarray's dependency on approx from 0.4 to 0.5 in ndarray 0.16.0 (i.e. a breaking release), but we cannot make the change in ndarray 0.15.4 (i.e. a non-breaking release) without violating semantic versioning.

To clarify, approx's change from 0.4 to 0.5 was a breaking change, since it changed its num-complex dependency from 0.3 to 0.4, and approx exposes implementations of its traits for num-complex::Complex. num-complex's change from 0.3 to 0.4 was a breaking change since it changed its rand dependency from 0.7 to 0.8, and it exposes implementations of rand::distributions::Distribution for its types. rand's change from 0.7 to 0.8 was a breaking change for various reasons; see its changelog. This illustrates how breaking changes propagate to downstream crates and helps show why we should minimize breaking changes (since it takes a while for breaking changes to propagate). rand made a breaking release, so num-complex made a breaking release, so approx made a breaking release.

@maoe
Copy link

maoe commented Jul 19, 2021

To clarify, approx's change from 0.4 to 0.5 was a breaking change, since it changed its num-complex dependency from 0.3 to 0.4, and approx exposes implementations of its traits for num-complex::Complex. num-complex's change from 0.3 to 0.4 was a breaking change since it changed its rand dependency from 0.7 to 0.8, and it exposes implementations of rand::distributions::Distribution for its types. rand's change from 0.7 to 0.8 was a breaking change for various reasons; see its changelog.

I see. This makes sense. Thanks for the explanation.

@bluss
Copy link
Member

bluss commented Nov 18, 2021

Approx is a public dependency - meaning we use their types/traits in our public API. So any update to "them" is an update to our API.

@bluss
Copy link
Member

bluss commented Nov 18, 2021

We will add this, but this feature will also disappear as soon as ndarray 0.16 is released.

@bluss
Copy link
Member

bluss commented Nov 18, 2021

Thanks!

@bluss bluss merged commit 99a5cb1 into rust-ndarray:master Nov 18, 2021
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.

3 participants