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

Deprecate `flatmap` #5989

Open
p6rt opened this issue Jan 6, 2017 · 9 comments
Open

Deprecate `flatmap` #5989

p6rt opened this issue Jan 6, 2017 · 9 comments
Labels
RFC

Comments

@p6rt
Copy link

@p6rt p6rt commented Jan 6, 2017

Migrated from rt.perl.org#130520 (status was 'open')

Searchable as RT130520$

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 6, 2017

From @smls

So, Perl 6 has a built-in `flatmap` routine​:

  .flatmap({ ... })

Show of hands​: Who is reasonably sure, without testing it or looking
it up, which of the following four expressions it corresponds to?

  .flat.map({ ... })

  .map({ ... }).flat

  .flat.map({ ... }).flat

  .map({ slip ... })

And if Perl 6 regulars and core developers can't tell, do you think a
typical Perl 6 newbie will guess correctly, and that code reviewers
will notice when the wrong behavior was assumed?

Note that the four behaviors are the same (or at least easy to
confuse) for many simple cases - so it's quite possible to assume the
wrong one even after a quick test, if one didn't test the more
complicated inputs for which where they do differ.


Second strike​: Whoever wrote the p6doc entry
<https://docs.perl6.org/routine/flatmap> assumed the wrong behavior as
well, and no one else seems to have noticed or cared. (See my initial
report at <https://github.com/perl6/doc/issues/851>.)

Third strike​: Its behavior isn't actually tested in Roast. The single
Roast test that exists for it
<https://github.com/perl6/roast/blob/master/S03-metaops/hyper.t#L410>
merely tests that it is nodal, and doesn't distinguish between the
four possible behaviors listed above.

Fourth strike​: Whereas `deepmap` and `duckmap` differ from plain `map`
in how they iterate the input and how often they call the lambda,
`flatmap` is the same as `map` in those regards and differs, rather,
in how it collects the output. Users who take the naming scheme
"deepmap, duckmap, flatmap" as an indication that these are three of a
kind, will assume the wrong behavior for `flatmap`.


Is all that confusion (and the needless proliferation of the Perl 6
core setting) worth it, just to save typing a single character in a
specific circumstance?

Unless I'm missing something vital, I'd say it's hard to argue that
the answer is "yes".

I therefore suggest initiating the deprecation cycle in order to
eventually drop of this routine from the Perl 6 language and Rakudo
implementation.

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 6, 2017

From @zoffixznet

On Thu, 05 Jan 2017 21​:47​:20 -0800, smls75@​gmail.com wrote​:

So, Perl 6 has a built-in `flatmap` routine​:

\.flatmap\(\{ \.\.\. \}\)

Show of hands​: Who is reasonably sure, without testing it or looking
it up, which of the following four expressions it corresponds to?

And if Perl 6 regulars and core developers can't tell

What do you mean? I just raised my hand showing I was sure! And there
are 57 instances of its use in core code, so seems like I'm not the only one.

Second strike​: Whoever wrote the p6doc entry
<https://docs.perl6.org/routine/flatmap> assumed the wrong behavior as
well, and no one else seems to have noticed or cared. (See my initial
report at <https://github.com/perl6/doc/issues/851>.)

You seem to have a pretty good idea of what it does. Why not fix the docs
and ameliorate the confusion issue instead of letting that doc Issue rot?

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 6, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 10, 2017

From @LLFourn

I took a look through my codebase to see where I have used flatmap. Every
place I think I would prefer map({ }).flat on second look.

so +1

LL

On Fri, Jan 6, 2017 at 4​:47 PM Sam S. <perl6-bugs-followup@​perl.org> wrote​:

# New Ticket Created by Sam S.
# Please include the string​: [perl #​130520]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=130520 >

So, Perl 6 has a built-in `flatmap` routine​:

\.flatmap\(\{ \.\.\. \}\)

Show of hands​: Who is reasonably sure, without testing it or looking
it up, which of the following four expressions it corresponds to?

\.flat\.map\(\{ \.\.\. \}\)

\.map\(\{ \.\.\. \}\)\.flat

\.flat\.map\(\{ \.\.\. \}\)\.flat

\.map\(\{ slip \.\.\. \}\)

And if Perl 6 regulars and core developers can't tell, do you think a
typical Perl 6 newbie will guess correctly, and that code reviewers
will notice when the wrong behavior was assumed?

Note that the four behaviors are the same (or at least easy to
confuse) for many simple cases - so it's quite possible to assume the
wrong one even after a quick test, if one didn't test the more
complicated inputs for which where they do differ.

---

Second strike​: Whoever wrote the p6doc entry
<https://docs.perl6.org/routine/flatmap> assumed the wrong behavior as
well, and no one else seems to have noticed or cared. (See my initial
report at <https://github.com/perl6/doc/issues/851>.)

Third strike​: Its behavior isn't actually tested in Roast. The single
Roast test that exists for it
<https://github.com/perl6/roast/blob/master/S03-metaops/hyper.t#L410>
merely tests that it is nodal, and doesn't distinguish between the
four possible behaviors listed above.

Fourth strike​: Whereas `deepmap` and `duckmap` differ from plain `map`
in how they iterate the input and how often they call the lambda,
`flatmap` is the same as `map` in those regards and differs, rather,
in how it collects the output. Users who take the naming scheme
"deepmap, duckmap, flatmap" as an indication that these are three of a
kind, will assume the wrong behavior for `flatmap`.

---

Is all that confusion (and the needless proliferation of the Perl 6
core setting) worth it, just to save typing a single character in a
specific circumstance?

Unless I'm missing something vital, I'd say it's hard to argue that
the answer is "yes".

I therefore suggest initiating the deprecation cycle in order to
eventually drop of this routine from the Perl 6 language and Rakudo
implementation.

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 10, 2017

From @AlexDaniel

On 2017-01-05 21​:47​:20, smls75@​gmail.com wrote​:

So, Perl 6 has a built-in `flatmap` routine​:

.flatmap({ ... })

Show of hands​: Who is reasonably sure, without testing it or looking
it up, which of the following four expressions it corresponds to?

.flat.map({ ... })

.map({ ... }).flat

.flat.map({ ... }).flat

.map({ slip ... })

And if Perl 6 regulars and core developers can't tell, do you think a
typical Perl 6 newbie will guess correctly, and that code reviewers
will notice when the wrong behavior was assumed?

Note that the four behaviors are the same (or at least easy to
confuse) for many simple cases - so it's quite possible to assume the
wrong one even after a quick test, if one didn't test the more
complicated inputs for which where they do differ.

---

Second strike​: Whoever wrote the p6doc entry
<https://docs.perl6.org/routine/flatmap> assumed the wrong behavior as
well, and no one else seems to have noticed or cared. (See my initial
report at <https://github.com/perl6/doc/issues/851>.)

Third strike​: Its behavior isn't actually tested in Roast. The single
Roast test that exists for it
<https://github.com/perl6/roast/blob/master/S03-metaops/hyper.t#L410>
merely tests that it is nodal, and doesn't distinguish between the
four possible behaviors listed above.

Fourth strike​: Whereas `deepmap` and `duckmap` differ from plain `map`
in how they iterate the input and how often they call the lambda,
`flatmap` is the same as `map` in those regards and differs, rather,
in how it collects the output. Users who take the naming scheme
"deepmap, duckmap, flatmap" as an indication that these are three of a
kind, will assume the wrong behavior for `flatmap`.

---

Is all that confusion (and the needless proliferation of the Perl 6
core setting) worth it, just to save typing a single character in a
specific circumstance?

Unless I'm missing something vital, I'd say it's hard to argue that
the answer is "yes".

I therefore suggest initiating the deprecation cycle in order to
eventually drop of this routine from the Perl 6 language and Rakudo
implementation.

Another thing to note is that flatmap was working differently before the GLR.

<AlexDaniel> commit​: pre-glr,HEAD say ((1, 2), <a b>).flatmap(&uc).join('|');
<committable6> AlexDaniel, ¦«pre-glr»​: 1|2|A|B␤¦«HEAD»​: 1 2|A B
<AlexDaniel> commit​: pre-glr,HEAD say ((1, 2), <a b>).map(&uc).join('|');
<committable6> AlexDaniel, ¦«pre-glr,HEAD»​: 1 2|A B

To me it seems that it slipped through the cracks and nobody thought about it after the GLR.

So yes, if somebody thinks that flatmap should stay, please justify its existence.

Until then, +1.

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 11, 2017

From @smls

What do you mean? I just raised my hand showing I was sure! And there
are 57 instances of its use in core code, so seems like I'm not the
only one.

Okay, I get it, I shouldn't have tried to be cute in how I phrased that.

But do you really think I'm wrong in considering each of the four behaviors listed at the top as something that a reasonable Perl 6 user might expect a routine called `flatmap` to do, and that this presents a trap? Especially if the behavior of the similarly named `deepmap` leads one to guess wrong?

If a feature provides significant value, such a trap can be a fair price. But if `flatmap` has any value at all over chaining `map` and `flat`, it's not apparent.

Can it work marginally faster? If so, that might justify its use in core, but for a user-exposed builtin that seems like a weak rationale. I can't think of any other functionally redundant built-in in Perl 6 that exists purely for some slight performance benefit. The possibilities for a slippery slope would be endless​: Do we also need a `flatzip`, `flatrotor`, `grepmap`, etc?
If the Perl 6 language was designed from the ground up for squeezing out every last drop of performance (rather than for convenience/flexibility/extensibility), it would be a very different language.

Or was it introduced to avoid copying the whole list in memory between the `map` and `flat` step, back when `map` used to return a memoizing `List/Parcel` rather than a one-off `Seq` before the GLR?

You seem to have a pretty good idea of what it does. Why not fix the
docs
and ameliorate the confusion issue

I think the state of its documentation is a symptom of the confusion the routine provokes, not its cause.

Also, I only know what I found out by thoroughly testing the routine with Rakudo.

In order to properly update the docs, waiting for an explanation from the language designers / core developers would be prudent, I think, seeing how it has no meaningful tests in Roast isn't mentioned in the design docs.

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 11, 2017

From @zoffixznet

On Wed, 11 Jan 2017 03​:03​:29 -0800, smls75@​gmail.com wrote​:

Okay, I get it, I shouldn't have tried to be cute in how I phrased
that.

Right, it's a bit weird to base your argument on a fictional survey with results you made up :)

But do you really think I'm wrong in considering each of the four
behaviors listed at the top as something that a reasonable Perl 6 user
might expect a routine called `flatmap` to do

In a way, yeah. deepmap doesn't do a combination of .deep and .map called in a particular order. I'd expect .flatmap to do something that *can't* be replicated by putting a dot in its name to make two other method calls. But it does, so IMO .flatmap is kinda useless.

@p6rt
Copy link
Author

@p6rt p6rt commented Jul 31, 2017

From @zoffixznet

But do you really think I'm wrong

Nope. I agree it should be tossed.

There's one test in 6.c-errata that merely tests its nodality.

I'd say deprecate it in 6.d and toss it in 6.e. I listed that on the 6.d's TODO​: perl6/6.d-prep@2cc614b

@p6rt
Copy link
Author

@p6rt p6rt commented Sep 1, 2017

From @AlexDaniel

A lot of discussion drifted to this ticket for some reason​: Raku/doc#1428

Let's get ourselves back here.

On 2017-07-31 14​:40​:50, cpan@​zoffix.com wrote​:

But do you really think I'm wrong

Nope. I agree it should be tossed.

There's one test in 6.c-errata that merely tests its nodality.

I'd say deprecate it in 6.d and toss it in 6.e. I listed that on the
6.d's TODO​: https://github.com/perl6/6.d-
prep/commit/2cc614bde4bc5c69884a63c3ba972dfa680b312c

@p6rt p6rt added the RFC label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.