[Numpy-discussion] Where to discuss NEPs (was: Re: new NEP: np.AbstractArray and np.asabstractarray)

[Nathaniel Smith]

On Fri, Mar 9, 2018 at 11:51 AM, Stefan van der Walt
<stefanv at berkeley.edu> wrote:
> On Fri, 09 Mar 2018 17:00:43 +0000, Stephan Hoyer wrote:
>> I'll note that we basically used GitHub for revising __array_ufunc__ NEP,
>> and I think that worked out better for everyone involved. The discussion
>> was a little too specialized and high volume to be well handled on the
>> mailing list.
>
> A disadvantage of GitHub PR comments is that they do not track
> sub-threads of conversation, so you cannot "reply to" a previous concern
> directly.

Yeah, I actually find email much easier for this kind of complex
high-volume discussion. Even if lots of people don't use traditional
threaded mail clients anymore [1], archives are still threaded, and
the tools that make line-by-line responses easy and the ability to
split off conversations are both really helpful. (E.g., the way I
split this thread off from the original one :-).) The __array_ufunc__
discussion was almost impenetrable on GH, I think.

I admit though that some of this is probably just that I'm more used
to the email-based discussion workflow. Honestly none of these tools
are particularly amazing, and the __array_ufunc__ conversation would
have been difficult and inaccessible to outsiders no matter what
medium we used. It's much more important that we just pick something
and use it consistently than that pick the Most Optimal Solution.

[1] Meaning this, not gmail's threads:
https://en.wikipedia.org/wiki/Conversation_threading#/media/File:Nntp.jpg

> PRs also mix inline comments (that become much less visible after
> rebases and updates) and "story line" comments.  These two "modes" of
> commenting, substantive discussion around ideas, v.s. concerns about
> specific phrasing, usage of words, typos, content of code snippets,
> etc., may require different approaches.  It would be quite easy to
> redirect the prior to the mailing list and the latter to the GitHub PR.

I don't think we should worry about this. Fiddly detail comments are,
by definition, not super important, and generally make up a tiny
volume of the discussion around a proposal. Also in practice reviewers
are no good at splitting up substantive comments from fiddly details:
the review workflow is that you read through and as thoughts occur you
write them down, so even if you start out thinking "okay, I'm only
going to comment on typos", then half-way through some paragraph
sparks a thought and suddenly you're writing something substantive
(and I'm as guilty of this as anyone, maybe more so...). Asking people
to classify their comments and then chiding them for putting them in
the wrong place etc. isn't a good use of time. Let's just pick one
place for everything and stick with it.

> I'm also not too keen on repeated PR creation and merging (it splits up
> the PR discussion even further).  Why not simply hold off until the PEP
> is ready, and view the documents on GitHub?  The rendering there is just
> as good.

Well, if we aren't using PRs for discussion then multiple PRs are fine
:-). And merging changes quickly is helpful because it makes the
rendered NEPs page a single one-stop-shop to see all the latest NEPs,
no matter what their current status.

If we do use PRs for discussion, then I agree that we should try to
keep the PR open until the NEP is "done", to minimize the splitting of
discussion. This does create a bit of extra friction because it turns
out that "is this done?" is not something you can really ever answer
for certain :-). Even after PEPs are accepted they usually end up
getting some further tweaks once people start implementing them.
Sometimes PEPs get abandoned in "Draft" state without ever being
accepted/rejected, and sometimes a PEP that had been abandoned for
years gets picked up and finished. You can see this in the Rust RFC
guidelines too [2]; they specifically address the issue of post-merge
changes, and it sounds like their solution is that if a substantive
issue is discovered in an accepted RFC, then you have to create a new
"fixup" RFC, which then gets its own PR for discussion. I guess if
this were our process then __array_ufunc__ would have ended up with ~3
NEPs :-).

This is all doable -- every approach has trade-offs. But we should
pick one, so we can adapt to those trade-offs.

[2] https://github.com/rust-lang/rfcs#the-rfc-life-cycle

-n

-- 
Nathaniel J. Smith -- https://vorpus.org