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

introduce basic ops on Range and ValuesCount #63

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nkbelov
Copy link
Contributor

@nkbelov nkbelov commented Aug 1, 2023

This introduces some typical operations on Range and ValueCount, conforms them to std::ops::RangeBounds and provides macros to construct them with a syntax analogous to normal Rust ranges.

I'm a bit unsure about the macros for a number of reasons:

  1. they require a different syntax to make it possible to exclude the lower bound,
  2. they sometimes don't parse very well, requiring parentheses around some expressions like f64::INFINITY,
  3. rustfmt rewrites a=..=b into a = ..=b,

but, on the other hand, they drastically reduce the verbosity of the other ways to construct these types.

An alternative approach would be to allow construction as From<std::ops::Range> etc. and allow to exclude the lower bound by a separate method:

Range::from(0..=1).exclude_lower()

The various checks for infinites and NaNs on f64 are perhaps somewhat excessive, as these can't be expressed in JSON and thus stored in payloads, but these still can be passed into Range if the latter isn't constructed carefully and produce surprising query results.

@timvisee
Copy link
Member

timvisee commented Aug 2, 2023

Nice work on this. Thank you for the efforts.

Regarding ranges, I don't like that range!(a..=b) now excludes a. It differs with Rusts ranges which is confusing.

Maybe we can come up with a different syntax for excluding. a-..=b or a!..=b could work, though that might look a bit weird. Adding exclude_lower() may end up being the least confusing.

@nkbelov
Copy link
Contributor Author

nkbelov commented Aug 2, 2023

After some thought I indeed too feel that the macros will be more confusing than helpful.

I have updated the code to support the exclude_lower() approach and construction from stdlib's ranges.

@nkbelov nkbelov changed the title introduce basic ops on Range and ValueCount introduce basic ops on Range and ValuesCount Aug 3, 2023
@nkbelov nkbelov closed this Aug 21, 2023
@nkbelov nkbelov reopened this Aug 21, 2023
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.

None yet

2 participants