-
Notifications
You must be signed in to change notification settings - Fork 40
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
Preserving static size information in no_offset_view
#301
Comments
I know people dislike Requires for many reasons but that seems unavoidable. -- since StaticArrays is already heavy enough, maybe it doesn't matter much to add an extra Requires dependency to StaticArrays? But this seems to be a marginal improvement to me -- I can't imagine how would people utilize this in practice. Perhaps we can just leave this issue here until someone really finds it a performance bottleneck? I'm not sure. |
Yes, this isn't urgent, as it's only a performance improvement. Maybe a |
* fix IdOffsetRange kwargs constructor * preserve types if indices are 1-based * no_offset_view in values * remove test for SOneTo (#301) * Add tests * bump version to v1.12.4
Perhaps this is worth considering in v1.9+ as a package extension |
Currently,
no_offst_view
has special methods defined forBsae.OneTo
, but nothing forSOneTo
axes that aStaticArray
has. As a consequence,OffsetArrays
doesn't realize that aSOneTo
is 1-based. This leads tothat is, it loses the static size information. As a consequence,
One way to resolve this is to simply define
_no_offset_view
forUnion{Base.OneTo, StaticArrays.SOneTo}
over here, but this will require us to explicitly depend onStaticArrays
. The second way might be something like JuliaLang/julia#41946, where Base definesOneTo <: AbstractOneTo
, and we define methods forAbstractOneTo
in this package. However, it doesn't appear that there's a clear direction to this inBase
at the moment. I wonder if there's another way to preserve the size, without one package explicitly depending on the other.The text was updated successfully, but these errors were encountered: