Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
add pure kwarg to map #71
add pure kwarg to map #71
Changes from 7 commits
6056598
62dcdb8
6745eef
2de3717
a3de2c3
6be6bde
065629d
73ff1ed
819bddf
4aef8d9
b3dc12c
eb5f5ee
9845443
ef7b491
de11e9e
5d80486
14bc92b
75aacf9
ef02b27
ebc4d0a
f1b60b1
d9a7e4a
60fdbfc
1716888
8aab260
a6c6e65
a59e22a
2f80224
3a0b7e8
3a97346
72a6089
09c2f20
ade7029
6acf2e8
a059bef
6a4bfa5
950d914
5c13102
4204992
aba380c
60efc6f
b98ee8f
a146fa6
97fe088
0b21c63
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@quinnj and @nalimilan - just to double check. Are we sure that
PooledArray
always uses 1-based indexing and even if it is multidimensional it can use a linear index?I recall @nalimilan recently giving some comment that potentially a non-standard
refs
field could be used. On the other handeachindex(IndexLinear(), ::PooledArray)
falls back to the default implementation:I will have a look into it later if you do not have an immediate answer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that in theory any
AbstractArray
type can be used asrefs
, but in practice we probably haven't checked that things work for non-1-based indexing, and the fallbacks you show assume 1-based indexing, right? So I'd say it's OK to assume 1 for now and fix that later if somebody complains.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok - then I have added an appropriate check in the inner constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a few
@inferred
checks where applicable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add the
@inferred
tests elsewhere, as here nothing is inferred since now inference produces theUnion
of two concrete types, see:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added
@inferred
withUnion
formap
tests