Skip to content

Implement LowerBounded and UpperBounded for Ratio#138

Open
tyilo wants to merge 1 commit into
rust-num:mainfrom
tyilo:bounded
Open

Implement LowerBounded and UpperBounded for Ratio#138
tyilo wants to merge 1 commit into
rust-num:mainfrom
tyilo:bounded

Conversation

@tyilo

@tyilo tyilo commented May 22, 2025

Copy link
Copy Markdown

No description provided.

@cuviper cuviper closed this Jul 6, 2026
@cuviper cuviper reopened this Jul 6, 2026
@cuviper

cuviper commented Jul 6, 2026

Copy link
Copy Markdown
Member

I wonder, is there a reason you chose these one-sided bounds rather than just Bounded?

@mikem8891

Copy link
Copy Markdown

I am pretty sure it is related to BigUint. The pull rust-num/num-bigint#330 makes impl LowerBounded for BigUint but not UpperBound, so I think impl<T: Bound> Bound for Ratio<T> would exclude BigUint and prevent impl<T: LowerBound> LowerBound for Ratio<T> because there is already a blanket impl in num-traits. Ideally, I think Bound would be trait Bound: LowerBound + UpperBound {} instead of having a blanket impl for UpperBound and LowerBound, but I don't know all the details of why it was implemented this way.

A compromise might be impl<T: Bound> Bound for Ratio<T> and impl LowerBound for Ratio<BigUint>, but I haven't tried it and the second impl needs to be behind the num-bigint feature flag.

@cuviper

cuviper commented Jul 7, 2026

Copy link
Copy Markdown
Member

Yeah, there's a FIXME in num-traits about changing to a supertrait (if we ever do a breaking change there). It's just a historical artifact that Bounded came first, on its own.

In the current places where we do use T: Bounded ourselves, we only call max_value(). We could switch those to T: UpperBounded, which is even semver-safe since there's a blanket impl in that direction. I doubt there's any real T: Integer that has an upper and not lower bound though.

impl LowerBound for Ratio<BigUint>

This is basically just Zero though -- is Ratio<T>: LowerBounded otherwise useful in some generic context?

FWIW, the original num-traits motivation was about ranges: rust-num/num-traits#208

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