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.

Sign up for GitHub

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

Jump to bottom

Feature: total completion. #480

Closed
wants to merge 2 commits into from
Closed

Feature: total completion. #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dab9225
Add `vectorToMaybe`.
kindaro Jan 25, 2024
b7d2f4d
Add property check for `vectorToMaybe`.
kindaro Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion vector/src/Data/Vector/Generic.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module Data.Vector.Generic (
length, null,

-- ** Indexing
(!), (!?), head, last,
(!), (!?), head, vectorToMaybe, last,
Copy link
Contributor

@lehins lehins Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I thought everyone was pretty clear that this is not a good name!

Suggested change
(!), (!?), head, vectorToMaybe, last,
(!), (!?), head, headMaybe, last,

Copy link
Author

Choose a reason for hiding this comment

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

This is not the right tone to talk to me in. Let us all stay friendly and respectful.

As I said in #479, we shall think about names for a little longer. Maybe we could even get some more evidence.

Copy link
Author

Choose a reason for hiding this comment

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

Actually, @lehins @konsumlamm @Shimuuar why exactly are you against vectorToMaybe? Neither of you have ever given any argument specifically against this choice. It is perfectly consistent with base because it is a direct parallel to listToMaybe from Data.Maybe. I am eager to find out about your reasoning!

Copy link
Contributor

Choose a reason for hiding this comment

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

vectorToMaybe doesn't really tell you anything, apart from what you can already see from the type signature (namely that its type is Vector a -> Maybe a). It might as well return the last element, or return Nothing if the list has more than one element. listToMaybe is not well known IME and I think one of the reasons is that listToMaybe is not the name most people would expect.

headMaybe on the other hand immediately makes me think of a function that returns the first element wrapped in a Maybe (and Nothing if there are no elements).

Copy link
Author

Choose a reason for hiding this comment

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

I agree that specificity is important. I talked about it in #479 (comment).

However, any name that mentions Maybe is also nonsense, and for the same reason: we should not write in the name what is already in the type. We need to come up with something better.

Copy link
Author

Choose a reason for hiding this comment

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

I am not trying to be rude. I am just saying how it is. It's just not gonna happen, at least as long as I am a maintainer of vector.

This sounds odd. Would you say adding 35 handy functions among which one is named not to your liking — but still reasonably and consistently with base — brings negative value to the Haskell ecosystem? Being so strongly set in a view for which a strong argument is unlikely to be found does not seem reasonable. This sounds more like «l'État, c'est moi».

I see no proof that either you or Aleksey have overwhelming expertise in naming things.

You know nothing about our experience and we don't need to prove anything to you.

I know you have great experience working on vector and random. But naming things is hard. It often boils down to personal preference. So, as I see it, it is very important to seek evidence. I wish we had an expert among us who could pick great names — alas, there is not one.

You seemed to imply that maintainers (some or all) have privileged insight into goodness of names. This statement incurs a burden of proof. Without it, «maintainers of the library told you on a couple of occasions that it is not a good name» has the same meaning as «someone told you on a couple of occasions that it is not a good name». This latter does contribute to statistics, but does not justify statements like «I will have to close the PR».

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you say adding 35 handy functions among which one is named not to your liking — but still reasonably and consistently with base — brings negative value to the Haskell ecosystem?

I would say try to add at least one function according to the maintainers' guidelines then maybe we can speak about the value you are bringing to Haskell ecosystem. So far, all I've seen you add is a whole lot of arguing.

You seemed to imply that maintainers (some or all) have privileged insight into goodness of names.

Yes. By definition it is our job to say yes or no to what goes into the repo. So it's up to maintainers to decide not only on functionality that get's merged, but also on names of functions.

It is my unpaid job to make decisions like that. If you don't like it, take it up with the management.

But for the love of god, could you please stop wasting my time with arguing?

In the matter of fact, my patience is running pretty thin, so if there is even one more argument form you about this topic, I will close this PR and will personally ignore anything else that comes from you.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, as I see it, it is very important to seek evidence.

https://www.stackage.org/lts-22.7/hoogle?q=%5Ba%5D+-%3E+Maybe+a suggests:

  • smthToMaybe,
  • headMay,
  • safeHead,
  • headMaybe.

I do insist on head being an infix of the name, because otherwise you cannot know which element of a non-empty vector is returned. headMaybe seems the best option to me, but headMay or safeHead could also work out.

You seemed to imply that maintainers (some or all) have privileged insight into goodness of names. 

Maintainers do not necessarily pretend to have a privileged insight or superior judgement about the absolute truth, but they do have a privileged access to the repository. Following maintainers' suggestions does not necessarily imply that you agree with them or find their views reasonable, if this constitutes a difficulty for you, it's just a certain way to get things moving forwards. If your purpose is the latter, I'd suggest we do not focus too much on options which are unlikely to gain maintainers' support.

(Fair enough if your actual purpose is to seek an absolute truth on the topic, but neither @lehins nor I have much interest in this direction, so we would not be inclined to participate much)

Copy link
Author

@kindaro kindaro Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@Bodigrim

  • (needs answer)   Yep, I looked at Hoogle and saw the same 4 variants a few days back. Do you also insist that we pick one of the names already on this list? None of them seems excellent to me. What about maybeHead? There is a lot of functions on Hackage that are called «maybe something» and return a Maybe, like say maybeAt, maybeCompatible, maybeDividedBy, maybeEntropy… Even maybeHead. There is even an example from ghc. This principle of naming will look good in more complicated combinations like maybeMaximumBy as well.

  • (for your information)   I agree that it is better to have head somewhere in the name, though my reasoning is rather that the name of the completion better be a variation on the name of the definition being completed.

  • (for your information)

    You seemed to imply that maintainers (some or all) have privileged insight into goodness of names.

    Maintainers do not necessarily pretend to have a privileged insight or superior judgement about the absolute truth, but they do have a privileged access to the repository. Following maintainers' suggestions does not necessarily imply that you agree with them or find their views reasonable, if this constitutes a difficulty for you, it's just a certain way to get things moving forwards. If your purpose is the latter, I'd suggest we do not focus too much on options which are unlikely to gain maintainers' support.

    Why mince words. If you compel another to act against their best judgement, this is called enforcement, not suggestion. See:

    suggest
    1
    a
    : to mention or imply as a possibility
    suggested that he might bring his family
    b
    : to propose as desirable or fitting
    suggest a stroll
    c
    : to offer for consideration or as a hypothesis
    suggest a solution to a problem

    So, it is me who is suggesting, and it is the maintainers who are (sometimes) enforcing. Other than that, I agree with everything you said.

    (Fair enough if your actual purpose is to seek an absolute truth on the topic, but neither @lehins nor I have much interest in this direction, so we would not be inclined to participate much)

    This is fine. You can see what I said in my first message here: «we shall think about names for a little longer». That is all I wanted.

    I am sorry that this disaster of a conversation between me and Alexey happened. I am in pain because of it myself.

@lehins   So now you openly threaten me.

In the matter of fact, my patience is running pretty thin, so if there is even one more argument form you about this topic, I will close this PR and will personally ignore anything else that comes from you.

If you await for me to cower in fear before you, I shall disappoint you. You put me down for everyone to see — this is wrong.

I would say try to add at least one function according to the maintainers' guidelines then maybe we can speak about the value you are bringing to Haskell ecosystem. So far, all I've seen you add is a whole lot of arguing.

Aside from being openly disrespectful, this seems like a circular requirement. You will only listen to me after I obey you, is that right?

You seemed to imply that maintainers (some or all) have privileged insight into goodness of names.

Yes. By definition it is our job to say yes or no to what goes into the repo. So it's up to maintainers to decide not only on functionality that get's merged, but also on names of functions.

You misunderstand me. Do you understand what epistemic privilege is? Hopefully we can agree that responsibility for or authority over something does not imply privileged insight into that thing. I say nothing about the former but I do ask you to justify the latter.

But for the love of god, could you please stop wasting my time with arguing?

I only answer to your messages and try to undo what I see as misunderstanding. Surely you were free to wrap the conversation up at any time. I am not responsible for the way you choose to spend your time — you are. I have not even asked for your review — this pull request is marked as draft.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why mince words. If you compel another to act against their best judgement, this is called enforcement, not suggestion.

@lehins cannot enforce you to do anything. What he can is to suggest which ways of collaboration will be productive from his point of view. If this knowledge does not shift your judgement - fine, an outcome is likely to be unproductive, but the choice and free will are still yours.

@kindaro we indeed live in the age when ending text with a period is considered rude, but sometimes exclamation mark is just an exclamation mark. Please consider a possibility that not everyone in the world dreams about insulting you with punctuation.

I'm sorry that you felt scared and threatened. I know the feeling when it seems that everyone is looking for an opportunity to damage you. Truth it that the majority of people do not keep you that close to their heart to bother with attacking your persona.

Feel free to contact me anytime.

unsafeIndex, unsafeHead, unsafeLast,

-- ** Monadic indexing
Expand Down Expand Up @@ -244,10 +244,21 @@ v !? i | i `inRange` length v = case basicUnsafeIndexM v i of Box a -> Just a


-- | /O(1)/ First element.
--
-- Partial.
-- Undefined at @fromList [ ]@.
-- Completion is 'vectorToMaybe'
head :: Vector v a => v a -> a
{-# INLINE_FUSED head #-}
head v = v ! 0

-- | /O(1)/ Just the first element — or nothing when there are none.
--
-- Completes 'head'.
vectorToMaybe :: Vector v a => v a -> Maybe a
{-# INLINE_FUSED vectorToMaybe #-}
vectorToMaybe v = v !? 0

kindaro marked this conversation as resolved.
Show resolved Hide resolved
-- | /O(1)/ Last element.
last :: Vector v a => v a -> a
{-# INLINE_FUSED last #-}
Expand Down
3 changes: 2 additions & 1 deletion vector/tests/Tests/Vector/Property.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ testPolymorphicFunctions _ = $(testProperties [
'prop_length, 'prop_null,

-- Indexing
'prop_index, 'prop_safeIndex, 'prop_head, 'prop_last,
'prop_index, 'prop_safeIndex, 'prop_head, 'prop_vectorToMaybe, 'prop_last,
'prop_unsafeIndex, 'prop_unsafeHead, 'prop_unsafeLast,

-- Monadic indexing (FIXME)
Expand Down Expand Up @@ -241,6 +241,7 @@ testPolymorphicFunctions _ = $(testProperties [
prop_createT = (\v -> V.createT (T.mapM V.thaw v)) `eq` id

prop_head :: P (v a -> a) = not . V.null ===> V.head `eq` head
prop_vectorToMaybe :: P (v a -> Maybe a) = not . V.null ===> V.vectorToMaybe `eq` (Just . head)
prop_last :: P (v a -> a) = not . V.null ===> V.last `eq` last
prop_index = \xs ->
not (V.null xs) ==>
Expand Down
Loading