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

ProjectTo is too permissive? #121

Open
mzgubic opened this issue Aug 1, 2022 · 3 comments
Open

ProjectTo is too permissive? #121

mzgubic opened this issue Aug 1, 2022 · 3 comments

Comments

@mzgubic
Copy link
Contributor

mzgubic commented Aug 1, 2022

Maybe this should be an error?

julia> using AxisKeys

julia> ka = wrapdims(rand(3, 2), obs=1:3, point=["a", "b"]);

julia> kb = wrapdims(rand(3, 2), obs=1:3, point=["c", "d"]);

julia> ProjectTo(ka)(kb)
2-dimensional KeyedArray(NamedDimsArray(...)) with keys:
   obs  3-element UnitRange{Int64}
   point  2-element Vector{String}
And data, 3×2 Matrix{Float64}:
      ("a")      ("b")
 (1)   0.928905   0.705413
 (2)   0.493408   0.997449
 (3)   0.877819   0.816695

We probably want to think about similar things as in invenia/NamedDims.jl#201 (comment)

@mcabbott
Copy link
Owner

mcabbott commented Aug 1, 2022

Could be. Unlike NamedDims there's some cost here to checking the key-vectors each time.

I'm not wholly convinced that this thing ought to exist. We could just accept a plain array as the gradient of a fancy one.

@mzgubic
Copy link
Contributor Author

mzgubic commented Aug 1, 2022

Yeah, fair, checking the keys is a performance/correctness checking tradeoff. Maybe we could still check dimnames by deferring to NamedDims though?

@mcabbott
Copy link
Owner

mcabbott commented Aug 1, 2022

Yes. I guess the style of this package would be to un-wrap & call the projector for the underlying NamedDimsArray?

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

No branches or pull requests

2 participants