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

setindex! for mappedarray(f, finv, A, B, C...) (and deprecate f_finv tuple) #21

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

timholy
Copy link
Member

@timholy timholy commented Jul 5, 2018

This rounds out the functionality for multi-mapping supporting m[i] = v for m = mappedarray(f, finv, A, B, C...).

This also switches the writable interface from mappedarray((f, finv), A, ...) to mappedarray(f, finv, A, ...). This seems necessary in order to ensure inferrability if f or finv is a type. The only downside I see is that since any object can be made callable, this really commits at the API level to insisting that A::AbstractArray in order to be able to reliably resolve intent.

@codecov-io
Copy link

codecov-io commented Jul 5, 2018

Codecov Report

Merging #21 into master will decrease coverage by 6.06%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
- Coverage     100%   93.93%   -6.07%     
==========================================
  Files           1        1              
  Lines          35       66      +31     
==========================================
+ Hits           35       62      +27     
- Misses          0        4       +4
Impacted Files Coverage Δ
src/MappedArrays.jl 93.93% <91.66%> (-6.07%) ⬇️

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 a03e5f3...da10a27. Read the comment docs.

This is motivated by preserving inferrability when passing types for
either f or finv.
@timholy timholy force-pushed the teh/multimapped branch 2 times, most recently from 05fb990 to b04ec1e Compare July 5, 2018 15:04
@timholy timholy merged commit cc35144 into master Jul 9, 2018
@timholy timholy deleted the teh/multimapped branch July 9, 2018 20:12
@Cody-G
Copy link

Cody-G commented Sep 14, 2018

There used to be a version of the teh/multimapped branch that supported multi-mapped arrays in Julia 0.6. Is there any chance that's still around somewhere? I have some code that relies on it.

@Cody-G
Copy link

Cody-G commented Sep 14, 2018

I rebased and got something working in 0.6. If there's any interest it's available at

https://github.com/Cody-G/MappedArrays.jl/tree/multimap_06

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.

3 participants