-
Notifications
You must be signed in to change notification settings - Fork 21
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
splitobs
make views, but say it requires only getobs
#167
Comments
Looking back through the blame, I'm guessing the idea was to not materialize large datasets. The easiest path would be to change the docs, but that doesn't solve the OneHotArrays issue. Maybe Lines 224 to 227 in af7ebea
obsview should be returning ObsView for more types?
|
Yes I'm sure not materialising was the goal. However, some views are more useful than others. I wonder if the default should be something like The OneHotArrays issue could be solved on that side, by changing what |
Regardless of solution, the docstring should 100% be updated to mention some type of view is returned with
How straightforward do you think this would be? Since FluxML/OneHotArrays.jl#40 is mostly about performance, another idea would be adding a kwarg which controls whether a view or copy is returned. |
The |
splitobs
and DataLoader
make views, but say they require only getobs
splitobs
make views, but say it requires only getobs
It's surprising that
splitobs
andDataLoader
make views, when they mention onlygetobs
in their docstrings, which does not:This means that they do not preserve OneHotArrays, which is FluxML/OneHotArrays.jl#40 .
But more generally, perhaps copies are just safer?
The text was updated successfully, but these errors were encountered: