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

Repeated names #148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Repeated names #148

wants to merge 1 commit into from

Conversation

mcabbott
Copy link
Collaborator

Maybe it's time to define & test the behaviour with repeated indices. This fixes:

  • sum(A, dims=:i) to use the first dimension named :i, as it already did, and
  • A[i=1] is an error if more than one dimension is named :i. Previously it fixed all such dimensions.

The one exception I can think of is that, for a Diagonal matrix, it may make sense to index both dimensions together. But maybe I have overlooked other uses?

The indexing change did not break any tests. But may still count as a breaking change, not sure.

@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #148 (1d3f4c7) into master (1ef0b73) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
+ Coverage   93.71%   93.76%   +0.04%     
==========================================
  Files          10       10              
  Lines         382      385       +3     
==========================================
+ Hits          358      361       +3     
  Misses         24       24              
Impacted Files Coverage Δ
src/name_core.jl 95.55% <100.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ef0b73...1d3f4c7. Read the comment docs.

@oxinabox
Copy link
Member

I know @nickrobinson251 likes for covarience matrixes that indexing with just the keyword gives you on the diagonal.
Not enough to use undefined behavour, but he thinks it is cute

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