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

Prelude: Add List.partition and Natural.{max,maximum,min,minimum,sort} #774

Merged
merged 4 commits into from Oct 21, 2019

Conversation

sjakobi
Copy link
Collaborator

@sjakobi sjakobi commented Oct 9, 2019

No description provided.

@sjakobi
Copy link
Collaborator Author

sjakobi commented Oct 9, 2019

sort can quite certainly be optimized further.

@Profpatsch
Copy link
Member

min/minimum and max/maximum is not a very human-friendly naming scheme.

@sjakobi
Copy link
Collaborator Author

sjakobi commented Oct 9, 2019

min/minimum and max/maximum is not a very human-friendly naming scheme.

Works for me I guess – and I'm human. But please propose better names! :)

Copy link
Contributor

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

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

Note that Prelude.Natural.sort is O(N^2) so in practice it will only handle lists of up to a few-thousand Natural numbers, but we can optimize it in a follow-up change

Prelude/Natural/sort Outdated Show resolved Hide resolved
Prelude/Natural/max Show resolved Hide resolved
… as suggested by @Gabriel439.
@Profpatsch
Copy link
Member

Profpatsch commented Oct 11, 2019

Works for me I guess – and I'm human. But please propose better names! :)

listMax and listMin e.g.

This is a special case of the Max and Min semigroups.

@sjakobi
Copy link
Collaborator Author

sjakobi commented Oct 11, 2019

listMin is admittedly clearer than minimum, but it also sounds quite a bit less nice or elegant to me.

Any other opinions on this?

@Profpatsch
Copy link
Member

but it also sounds quite a bit less nice or elegant to me.

At least it’s not confusing ;)

@Gabriella439
Copy link
Contributor

I personally liked the current naming scheme (min/max/minimum/maximum), if only because it's consistent with Haskell's naming scheme

@Profpatsch
Copy link
Member

Profpatsch commented Oct 11, 2019

I mean it’s pretty easy to discern by its type, which I guess is what motivated the naming in Haskell in the first place.

@joneshf
Copy link
Collaborator

joneshf commented Oct 12, 2019

What about renaming to:

  • Prelude/List/maximum
  • Prelude/List/minimum
  • Prelude/Natural/maximum
  • Prelude/Natural/minimum
    or:
  • Prelude/List/max
  • Prelude/List/min
  • Prelude/Natural/max
  • Prelude/Natural/min
    ?

@sjakobi
Copy link
Collaborator Author

sjakobi commented Oct 12, 2019

@joneshf We will probably soon be able to compare other types apart from Natural, for example Integer. So a List.maximum should IMHO not be specialized to Naturals.

@joneshf
Copy link
Collaborator

joneshf commented Oct 12, 2019

What would a Prelude/List/maximum be in that world?

@sjakobi
Copy link
Collaborator Author

sjakobi commented Oct 12, 2019

What would a Prelude/List/maximum be in that world?

It could take a comparison function:

forall (a : Type) -> (a -> a -> Bool) -> List a -> Optional a

or

forall (a : Type) -> (a -> a -> Ordering) -> List a -> Optional a

@neongreen
Copy link

if only because it's consistent with Haskell's naming scheme

This doesn't sound like a good guideline to use if we want Dhall to take over the non-Haskell world. (Whether we do is a different question though.)

@sjakobi sjakobi merged commit 7f455c1 into master Oct 21, 2019
@sjakobi sjakobi deleted the sjakobi/min-etc branch October 21, 2019 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants