gleam-community / maths Goto Github PK

I noticed that round returns a Result(Float, String), but I couldn't see anywhere in the docs when/why this can return an error (or what the error is). Would be great to have this documented!

The functions in the combinatorics and sequences modules could be improved by making them return an iterator instead of a list, which will be prohibitively expensive to do for larger inputs. See related comments and info by DanielSherlock:

No worries, I'm not in any rush (and I hope you aren't too)!

Actually, I thought after posting this pull request: Oh before I start writing tests and docs I should go and look at the standard (or standard-ish) libraries of other languages to see how they handle this case. Then I got distracted. And now it is now.

As far as I can tell, it's like this:

(I had hoped to find a list-permuting function in the standard (or standard-ish) libraries of Erlang or Elixir, and prioritise similarity to that. But no luck, not even Elixir's Math library has this. Other languages that seem to have nothing (semi-)official include Java, Elm, and Lobster. Though perhaps I wasn't looking hard enough.)

Personally, I like the way Swift handles it. Plus, their documentation is clear (with examples) and also has some hints for possible efficient implementations. The Clojure implementation seem nice and clean too, and might be easy to port. Not all other documentation was similarly clear...

Another thing to note: Almost all of the languages I surveyed returned an iterator (or equivalent thing in that language), even when given a list/array/etc. If we are going to be worried about performance, we might consider doing the same. This after all exactly the sort of function where iterators make a lot of sense:

• It does not take a particularly large input list for the output list to be very very large indeed.
• The state needed to generate the next permutation is small.

Originally posted by @DanielSherlock in #9 (comment)

The way the code is organised at the moment, it becomes impossible to use some modules alongside the same-named ones in the stdlib without some aliasing. It seems quite reasonable to want to import both gleam/int and gleam_community/maths/int and having to alias one is quite clunky.

I'd much prefer to see things organised by domain/use than type, for example a gleam_community/maths/trigonometry module: trigonometry.asin feels a lot more appropriate to me than float.asin.

For functions that are defined both ints and floats, like sign I'd rather see those functions live in the same place with some established naming convention like int_sign and float_sign, or sign and signf, or whatever...

For the list_ modules consider whether the API can be simplified by simply asking consumers to provide implementations of some behaviour, instead of int_list.maximum([...]) and float_list.maximum([...]) this could be unified maths.maximum and the user can provide int.compare, float.compare, or float.loosely_compare as they see fit.

I'd consider the requirement to alias imports to use the package a significant blocker for an otherwise very useful package, but I'm not familiar enough with the domain to provide specific suggestions for re-organising things. Hopefully this is enough to point you in the right direction ❤️

When I see errors like "Invalid rounding mode. Valid input is 'Nearest', 'TiesAway', 'TiesUp', 'ToZero', 'Down', 'Up'." that's a good indicator to me that we could probably use a custom type instead and eliminate the impossible states entirely!

If we instead add...

pub type RoundingMode {
  Nearest
  TiesAway
  TiesUp
  ToZero
  Down
  Up
}

...we get compile-time errors if we have a typo or use an unsupported mode, and now we don't have to handle a catch-all case for other arbitrary strings.

Fix the last couple of ToDos, review the code and publish as package on hex.

There are at least two todos in the package and a handful in the tests! Just opening this up so we don't forget about it:

• list.combination
• list.do_permutation
• the tests for both of these

^.^

After installing this library, not using it in my code, and running my project, I get this:

gleam run
  Compiling gleam_community_maths
error: Unknown module field
    ┌─ /Users/volkmannm/Documents/dev/lang/gleam/demo_gleam/build/packages/gleam_community_maths/src/gleam_community/maths/piecewise.gleam:945:33
    │
945 │       let assert Ok(val0) = list.at(arr, 0)
    │                                 ^^^ Did you mean `last`?

The module `gleam/list` does not have a `at` value.