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

WIP: Stash axes into indices #58

Closed
wants to merge 1 commit into from
Closed

Conversation

mbauman
Copy link
Member

@mbauman mbauman commented Feb 17, 2017

Does not fully work yet, but I think this may be a viable approach. It makes things like broadcasting "just work". Not sure when I will get a chance to finish this... I have been slowly iterating bit by bit over the past few weeks.

Does not fully work yet, but I think this may be a viable approach. It makes things like broadcasting "just work". Not sure when I will get a chance to finish this... I have been slowly iterating bit by bit over the past few weeks.
@timholy
Copy link
Member

timholy commented Feb 17, 2017

I'd been toying with this notion too. Excited that you tried it! I cloned this and will start playing with it.

@mbauman
Copy link
Member Author

mbauman commented Feb 17, 2017

It does make the Axis type rather strange — it becomes an AbstractUnitRange, acting like the linearindices of the value it wraps. Kinda. Right now I have ax[2] == 2 but ax[2:end] == OneTo(length(ax)-1) for a one-based axis, which is particularly bizarre.

But once you wrap your head around that bit of backwardness, it seems to make sense. Feel free to push directly to this branch.

@timholy
Copy link
Member

timholy commented Feb 20, 2017

I still like this idea in principle, but after playing with it a bit I'm concerned that the dual interpretations of "value" (as an AbstractUnitRange and whatever AbstractVector the user supplied) are at risk for serious confusion. For now I'm going to continue #57 using a more conservative approach, but this still merits further investigation.

@mbauman
Copy link
Member Author

mbauman commented Feb 20, 2017

Yeah, I think this is trying to do too much with the Axis type. I had been hoping that this would work… and that we could rename Base.indices() to Base.axes(), which would help with the too-many-indices problem when describing how the indices you use to index with must be within the indices of the array.

I may still try to make this work with an IndicesWrapper that does the backwards "indices of the axes are the values of the type" operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants