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

feat!: Update strum and ordered-float dependencies and change From<LogProb> into TryFrom<LogProb> for NotNan<f64>. #491

Merged
merged 11 commits into from Aug 19, 2022
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,12 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).


## [0.42.0](https://www.github.com/rust-bio/rust-bio/compare/v0.42.0...v0.41.0) (2022-05-03)

* Upgrade `strum` to version `0.24`
* Upgrade `ordered-float` to `2.0`

## [0.41.0](https://www.github.com/rust-bio/rust-bio/compare/v0.40.0...v0.41.0) (2022-03-30)


Expand Down
8 changes: 4 additions & 4 deletions Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "bio"
version = "0.41.0"
version = "0.42.0"
johanneskoester marked this conversation as resolved.
Show resolved Hide resolved
authors = ["Johannes Köster <johannes.koester@tu-dortmund.de>"]
description = "A bioinformatics library for Rust. This library provides implementations of many algorithms and data structures that are useful for bioinformatics, but also in other fields."
homepage = "https://rust-bio.github.io"
Expand Down Expand Up @@ -37,16 +37,16 @@ serde_derive = "1.0"
approx = ">=0.3, <0.6"
custom_derive = "0.1"
newtype_derive = "0.1"
ordered-float = "1.0"
ordered-float = "2.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we're updating this... would it be possible to make this

Suggested change
ordered-float = "2.0"
ordered-float = ">=2, <4"

There were breaking changes in 3.0.0, but if they don't break this crate then it would be preferable to allow users to decide which version they want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to me. @adam-azarchs are you sure that the breaking changes don't affect us here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100%. They don't look like behavior changes, just some dropped implementations, so it should be easy enough to just try it and see if it compiles. I know for our internal purposes we're not going to be updating to 3.0 particularly soon but future proofing is good.

regex = { version = "1.3", default-features = false, features = ["std", "perf"] }
multimap = ">=0.6, <0.9"
fxhash = "0.2"
statrs = ">= 0.11, < 0.16"
bio-types = ">=0.11.0"
pest = { version = "2", optional = true }
pest_derive = { version = "2", optional = true }
strum = ">= 0.16, < 0.24"
strum_macros = ">= 0.16, < 0.24"
strum = "0.24"
strum_macros = "0.24"
evolvedmicrobe marked this conversation as resolved.
Show resolved Hide resolved
getset = ">=0.0.9, <0.2"
enum-map = ">=0.6.4, <2"
triple_accel = ">=0.3, <0.5"
Expand Down
2 changes: 1 addition & 1 deletion src/pattern_matching/pssm/mod.rs
Expand Up @@ -296,7 +296,7 @@ pub trait Motif {
let bits = Self::get_bits();
let scores = self.get_scores();
let mut tot = 0.0;
for row in scores.genrows() {
for row in scores.rows() {
tot += bits - ent(row.iter());
}
tot
Expand Down
15 changes: 8 additions & 7 deletions src/stats/probs/adaptive_integration.rs
Expand Up @@ -5,7 +5,7 @@

use std::cmp;
use std::collections::HashMap;
use std::convert::Into;
use std::convert::{Into, TryFrom};
use std::hash::Hash;
use std::{
fmt::Debug,
Expand Down Expand Up @@ -43,7 +43,7 @@ use ordered_float::NotNan;
/// );
/// abs_diff_eq!(integral.exp(), 0.682, epsilon=0.01);
/// ```
pub fn ln_integrate_exp<T, F>(
pub fn ln_integrate_exp<T, F, E>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a PR changing the signature of a public method like this (which, granted, in most cases won't actually affect most users because the type will just be inferred, but will affect anyone who needs to specify any of the generic parameters) should probably not be labeled "chore", since "chore" PRs don't show up in the changelog.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you classify it at a build: then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be feat: actually, since the API is extended with a new feature here (allowing to have a fallible conversion from f64)

mut density: F,
min_point: T,
max_point: T,
Expand All @@ -57,10 +57,11 @@ where
+ Div<NotNan<f64>, Output = T>
+ Mul<Output = T>
+ Into<f64>
+ From<f64>
+ TryFrom<f64, Error = E>
+ Ord
+ Debug
+ Hash,
E: Debug,
F: FnMut(T) -> LogProb,
f64: From<T>,
{
Expand Down Expand Up @@ -112,7 +113,7 @@ where
// METHOD additionally investigate small interval around the optimum
for point in linspace(
cmp::max(
middle.unwrap() - (max_resolution.into() * 3.0).into(),
middle.unwrap() - T::try_from(max_resolution.into() * 3.0).unwrap(),
evolvedmicrobe marked this conversation as resolved.
Show resolved Hide resolved
min_point,
)
.into(),
Expand All @@ -124,23 +125,23 @@ where
linspace(
middle.unwrap().into(),
cmp::min(
middle.unwrap() + (max_resolution.into() * 3.0).into(),
middle.unwrap() + T::try_from(max_resolution.into() * 3.0).unwrap(),
max_point,
)
.into(),
4,
)
.skip(1),
) {
grid_point(point.into(), &mut probs);
grid_point(T::try_from(point).unwrap(), &mut probs);
}

let sorted_grid_points: Vec<f64> = probs.keys().sorted().map(|point| (*point).into()).collect();

// METHOD:
// Step 2: integrate over grid points visited during the binary search.
LogProb::ln_trapezoidal_integrate_grid_exp::<f64, _>(
|_, g| *probs.get(&T::from(g)).unwrap(),
|_, g| *probs.get(&T::try_from(g).unwrap()).unwrap(),
&sorted_grid_points,
)
}
2 changes: 1 addition & 1 deletion src/stats/probs/mod.rs
Expand Up @@ -403,7 +403,7 @@ impl From<NotNan<f64>> for LogProb {

impl From<LogProb> for NotNan<f64> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably also add a TryFrom impl here for people who want to acknowledge the fact that the conversion is fallible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's more of a larger change, but would be consistent with the reasoning in unordered-float

fn from(p: LogProb) -> NotNan<f64> {
NotNan::from(*p)
NotNan::new(*p).unwrap()
}
}

Expand Down