Rename endof->lastindex and introduce firstindex. Fixes #23354 #25458

timholy · 2018-01-08T19:39:20Z

While working on JuliaLang/Juleps#47 I noticed that we really should have beginindex (see #23354). This almost resolves that issue: I think I have all the tedious stuff done, but my lisp is laughable and character-twiddling was insufficient to fix the parsing of a[begin]. Would love some help. @stevengj? (Anyone who wishes, you are more than welcome to push directly to this branch, or do whatever you find most convenient.)

I defined specific beginindex methods for every type that defines what is now called endindex (formerly endof). Since beginindex is a new function, I also added a "deprecation" that returns 1 for any object, as a way of encouraging package authors to define it as needed. Hope this seems like a reasonable strategy.

mbauman · 2018-01-08T19:57:55Z

src/julia-syntax.scm

-          `(call (top endof) ,a)
-          `(call (top size) ,a ,n))
+          `(call (top endindex) ,a)
+          `(call (top last) (call (top axes) ,a ,n)))


I like this a lot, but could we incorporate this behavior into endindex, too? That way the parser only lowers end to endindex(A) or endindex(A, n) for the one- and n-arg cases, respectively.

stevengj · 2018-01-08T21:27:21Z

Can we just keep endof and call it beginof?

ararslan · 2018-01-08T21:28:19Z

or firstof?

timholy · 2018-01-08T21:47:30Z

Keeping it endof is of course an option, just not what appeared to be decided in #23354. I'm happy to go back to endof, though.

If we implement a[begin] I don't think it makes sense to call it firstof.

If someone gets to the parsing portion of this soon, great. If not, perhaps it would make sense to merge with beginof/beginindex and then implement a[begin] in 1.x.

timholy · 2018-01-08T21:48:23Z

Note that the only part of this that would have to be done for 1.0 is the rename, which is part of why I tackled this.

GregPlowman · 2018-01-08T21:49:21Z

Here's a user's perspective:

Can we just keep endof and call it beginof?

I think beginindex and endindex more clearly conveys that an index rather than an element is returned.
I used to get confused between endof(collection) and last(collection)

or firstof?

I don't think it ties in as nicely with a[begin]
#23354 (comment)

Sacha0 · 2018-01-08T22:13:00Z

Echoing @GregPlowman, the index names seem more descriptive.

Though firstindex/lastindex are less consistent with begin/end than beginindex/endindex, firstindex/lastindex are more consistent with first/last than beginindex/endindex. firstindex/lastindex also seem more descriptive/intuitive IMHO. (beginindex/endindex suggest performing "begin" and "end" operations on an index, whatever that would mean, rather than getting the first and last indices of a collection).

Intuitiveness in direct use of firstindex/lastindex (and particularly alongside first/last) seems like a more meaningful consideration than consistency between begin/end and the forms those lower to. Best! (Apologies for stoking the bikeshed!)

stevengj · 2018-01-09T00:03:20Z

One difficulty with parsing a[begin] is that some look ahead is required, since a[begin...end] is valid.

Maybe if begin is encountered in a ref expression, it should only be treated as an identifier if it is followed by an operator, comma, or ]?

timholy · 2018-01-09T00:08:45Z

Maybe also preceded? e.g. a[2*begin:end].

mbauman · 2018-01-09T00:21:23Z

When I had thought about doing this in the past, I was thinking we could just deprecate begin … end blocks within indexing altogether. We could also just break it entirely if there's enough weight to enabling this syntax relative to the number of uses of x[begin … end].

I really don't think we should try to support both begin/end as indices and begin … end blocks at the same time.

stevengj · 2018-01-09T00:24:27Z

@mbauman, yes, you are probably right. For example, a[begin +1; end] is currently valid syntax (equivalent to a[1]), but it means that we can't tell purely from the subsequent character whether begin is the start of a block or should be used as a variable.

yurivish · 2018-01-09T03:46:19Z

What would happen if the interior of an indexing expression contains a macro call that expands to a begin/end block, and would cases like a[f(begin 1 + 1 end)] still be allowed?

It feels like a very reasonable thing to just disallow begin...end blocks from syntactically appearing in the surface code so long as all the other ways they could come up continue to work.

mauro3 · 2018-01-09T09:56:02Z

Could we still rename begin ... end blocks? block ... end would be reasonable.

KristofferC · 2018-01-09T10:12:06Z

block ... end is quite appealing. for begins a for loop, if begins an if statement, block begins a block. begin is pretty non-saying, begin what?

stevengj · 2018-01-09T13:00:25Z

@yurivish, we are discussing the parser, which affects only surface syntax. Macro expansion happens later, and wouldn't be affected.

StefanKarpinski · 2018-01-09T14:58:51Z

I rather like endof and beginof but I certainly hear (and agree) with what @GregPlowman is saying about endindex and beginindex being much more clearly evocative of what they mean.

block ... end is not bad at all, but I also think that disallowing begin ... end inside of indexing is a pretty sane thing to do. This is already a bit of a problem since you cannot use the index meaning of end inside of a begin ... end block inside of an indexing expression (reasonably enough).

Keno · 2018-01-09T16:13:46Z

I don't have much to add here at the moment, except to add that in #25261 I ran into a bunch of problems with code assuming that the iteration and indexing states where identical (e.g. using start/endof to bound indices and next instead of nextindex), so I'm happy to see this.

stevengj · 2018-01-09T16:33:10Z

(The term begin here probably comes from Scheme's (begin ...) construct.) It seems quite annoying to rename a basic keyword of the language, and to introduce a new reserved word block, at this stage of the game.

timholy · 2018-01-09T16:39:41Z

I'm fine with any reasonable solution to the parsing issues.

StefanKarpinski · 2018-01-09T18:21:12Z

I think the best approach is probably to disallow begin ... end blocks inside of indexing in 0.7 and then introduce a[begin] as an analogue to a[end] in 1.0. The names beginindex and endindex seem fine to me.

JeffBezanson · 2018-01-17T18:39:35Z

I do prefer the names beginindex and endindex to the current names. If this PR is changed just to do those renamings, I can add a deprecation for begin ... end inside indexing, and then we can add a[begin] safely in 1.0.

part of #23354, #25458

Keno · 2018-01-22T21:20:41Z

Needs a rebase. If the syntax part of this is controversial, maybe we can at least get the beginindex parts? I need to separate indexing state from iteration before we can merge #25261, so this would be a good thing to base that on.

Keno · 2018-01-22T21:22:12Z

Actually I'll just go ahead, rebase this, and take out the syntax parts (hope you don't mind).

Keno · 2018-01-22T22:29:06Z

Rebased. I backed out the begin syntax as a separate commit, so whoever wants to take that up after 1.0 can just revert that change.

mbauman · 2018-01-22T23:45:16Z

doc/src/manual/interfaces.md

@@ -161,7 +161,8 @@ julia> collect(Iterators.reverse(Squares(10)))' # transposed to save space
 |:-------------------- |:-------------------------------- |
 | `getindex(X, i)`     | `X[i]`, indexed element access   |
 | `setindex!(X, v, i)` | `X[i] = v`, indexed assignment   |
-| `endof(X)`           | The last index, used in `X[end]` |
+| `beginindex(X)`         | The first index, used in `X[begin]` |


The syntax here is a holdover.

gabrielgellner · 2018-01-23T17:36:11Z

I find beginindex is really hard to scan due to the internal inin this feels like an future api wart to me.

StefanKarpinski · 2018-01-23T18:51:40Z

We could just call it begindex

fredrikekre · 2018-01-23T19:11:26Z

... and endex. begindex and endex seems like a nice couple 😄

KristofferC · 2018-01-23T19:54:11Z

begindex and endex seems like a nice couple

Too bad they are exes.

Keno · 2018-01-24T18:19:48Z

Rebased again. I'm planning to merge this one CI passes.

StefanKarpinski · 2018-01-24T19:02:31Z

Since beginindex is indeed a horrific name and when we discussed this previously, everyone agreed that firstindex and lastindex were the clearest names semantically, let's go with those instead. It's not so terrible that a[end] calls lastindex and a[begin] calls firstindex. lastindex is a far clearer name than endof for example, which was fairly opaque.

JeffBezanson · 2018-01-24T19:06:59Z

I agree. It's not overly confusing for a[end] to call lastindex. After all, it's also not obvious that a[end] would correspond to endof or endindex, or that . would call broadcast, etc.

Keno · 2018-01-24T21:48:22Z

Rebased and renamed.

ararslan · 2018-01-24T21:51:26Z

You might want to amend the commit message for the first commit here since what it describes is no longer quite what it does, since the names are different.

Keno · 2018-01-24T21:52:31Z

Done

StefanKarpinski · 2018-01-24T22:19:21Z

NEWS.md

@@ -991,6 +991,8 @@ Deprecated or removed

  * `Base.@gc_preserve` has been deprecated in favor of `GC.@preserve` ([#25616]).

+  * `endof(a)` has been renamed `endindex(a)` ([#23554]).


lastindex

Keno · 2018-01-25T22:57:59Z

Some idiot merged something that conflicts with this. I'll rebase again, sigh. Planning to merge next time we're green on CI.

It was decided to delay this syntax change until after 1.0, since it is non-breaking. Once somebody wants to take this up again, they can simply revert this commit.

Keno · 2018-01-25T23:11:09Z

People expressed concern about the phrasing, so for the record, it was me in #25733, I'm the idiot ;).

timholy · 2018-01-31T17:27:34Z

Thanks for picking this up and pushing it over the line, @Keno; it's been a busy few weeks.

mbauman reviewed Jan 8, 2018

View reviewed changes

JeffBezanson added the deprecation This change introduces or involves a deprecation label Jan 17, 2018

JeffBezanson added a commit that referenced this pull request Jan 17, 2018

disallow begin inside indexing

2a001c9

part of #23354, #25458

JeffBezanson mentioned this pull request Jan 17, 2018

disallow begin inside indexing #25614

Merged

JeffBezanson added a commit that referenced this pull request Jan 18, 2018

disallow begin inside indexing (#25614)

ae0a3e9

part of #23354, #25458

Keno added this to the 1.0 milestone Jan 22, 2018

Keno force-pushed the teh/beginindex branch from f5c5109 to 8f98b41 Compare January 22, 2018 22:27

mbauman reviewed Jan 22, 2018

View reviewed changes

Keno force-pushed the teh/beginindex branch from 8f98b41 to 968d381 Compare January 24, 2018 18:19

Keno force-pushed the teh/beginindex branch from 968d381 to bf3180a Compare January 24, 2018 21:47

ararslan changed the title ~~RFH: Rename endof->endindex and introduce beginindex. Fixes #23354~~ Rename endof->lastindex and introduce firstindex. Fixes #23354 Jan 24, 2018

Keno force-pushed the teh/beginindex branch from bf3180a to 29b098e Compare January 24, 2018 21:52

StefanKarpinski reviewed Jan 24, 2018

View reviewed changes

Keno force-pushed the teh/beginindex branch 2 times, most recently from cb6fd0e to c6de074 Compare January 25, 2018 01:20

timholy and others added 2 commits January 25, 2018 18:00

Rename endof->lastindex and introduce firstindex. Fixes #23354

742a539

Back out a[begin] syntax

e016f11

It was decided to delay this syntax change until after 1.0, since it is non-breaking. Once somebody wants to take this up again, they can simply revert this commit.

Keno force-pushed the teh/beginindex branch from e921daa to e016f11 Compare January 25, 2018 23:00

Keno merged commit 7d3991f into master Jan 26, 2018

ararslan deleted the teh/beginindex branch January 26, 2018 19:01

ararslan mentioned this pull request Feb 4, 2018

start keyword for array indexing #25873

Closed

ararslan mentioned this pull request Feb 14, 2018

Fix incorrect function name in deprecation #26047

Merged

stevengj mentioned this pull request Nov 25, 2019

support a[begin] for a[firstindex(a)] #33946

Merged

		@@ -991,6 +991,8 @@ Deprecated or removed

		* `Base.@gc_preserve` has been deprecated in favor of `GC.@preserve` ([#25616]).

		* `endof(a)` has been renamed `endindex(a)` ([#23554]).

Rename endof->lastindex and introduce firstindex. Fixes #23354 #25458

Rename endof->lastindex and introduce firstindex. Fixes #23354 #25458

Conversation

timholy commented Jan 8, 2018

mbauman Jan 8, 2018

Choose a reason for hiding this comment

stevengj commented Jan 8, 2018 • edited Loading

ararslan commented Jan 8, 2018

timholy commented Jan 8, 2018

timholy commented Jan 8, 2018

GregPlowman commented Jan 8, 2018

Sacha0 commented Jan 8, 2018 • edited Loading

stevengj commented Jan 9, 2018

timholy commented Jan 9, 2018

mbauman commented Jan 9, 2018

stevengj commented Jan 9, 2018

yurivish commented Jan 9, 2018 • edited Loading

mauro3 commented Jan 9, 2018

KristofferC commented Jan 9, 2018

stevengj commented Jan 9, 2018

StefanKarpinski commented Jan 9, 2018

Keno commented Jan 9, 2018

stevengj commented Jan 9, 2018 • edited Loading

timholy commented Jan 9, 2018

StefanKarpinski commented Jan 9, 2018

JeffBezanson commented Jan 17, 2018

Keno commented Jan 22, 2018

Keno commented Jan 22, 2018

Keno commented Jan 22, 2018 • edited Loading

mbauman Jan 22, 2018

Choose a reason for hiding this comment

gabrielgellner commented Jan 23, 2018

StefanKarpinski commented Jan 23, 2018

fredrikekre commented Jan 23, 2018

KristofferC commented Jan 23, 2018 • edited Loading

Keno commented Jan 24, 2018

StefanKarpinski commented Jan 24, 2018

JeffBezanson commented Jan 24, 2018 • edited Loading

Keno commented Jan 24, 2018

ararslan commented Jan 24, 2018

Keno commented Jan 24, 2018

StefanKarpinski Jan 24, 2018

Choose a reason for hiding this comment

Keno commented Jan 25, 2018

Keno commented Jan 25, 2018

timholy commented Jan 31, 2018

stevengj commented Jan 8, 2018 •

edited

Loading

Sacha0 commented Jan 8, 2018 •

edited

Loading

yurivish commented Jan 9, 2018 •

edited

Loading

stevengj commented Jan 9, 2018 •

edited

Loading

Keno commented Jan 22, 2018 •

edited

Loading

KristofferC commented Jan 23, 2018 •

edited

Loading

JeffBezanson commented Jan 24, 2018 •

edited

Loading