Skip to content
This repository was archived by the owner on May 23, 2022. It is now read-only.

remove ObsDims and data iterators #46

Merged
merged 9 commits into from
Jul 27, 2021
Merged

remove ObsDims and data iterators #46

merged 9 commits into from
Jul 27, 2021

Conversation

CarloLucibello
Copy link
Member

@CarloLucibello CarloLucibello commented Jul 25, 2021

Continuation of #44
(I'm a bit lazy to figure out how to push into another person PR right now so I'm opening this new PR)

Compared to #44, this

  • doesn't add the label-related functions
  • fixes tests,
  • improves some docs
  • removes a few abstract classes for data iteration that I think we can well live without

@darsnack
Copy link
Member

Thanks for taking this up. #44 is done; the only remaining blocker is JuliaML/MLDataPattern.jl#50. MLLabelUtils.jl PR (JuliaML/MLLabelUtils.jl#44) is also done.

Label-related functions were added cause that's what the comments for MLLabelUtils.jl indicated needed to be done. I'll give you write access to all my forks if you want to push the 3 PRs forward.

@CarloLucibello
Copy link
Member Author

I'm merging this as it is, since downstream packages updates should not be considered blockers, they will be ready to update their compat bounds when they will be ready.

Also, I'd like to leverage getobs and nobs as soon as possible to finally have a consistent dataset interface across the ecosystem.

@CarloLucibello CarloLucibello merged commit aae4cbd into master Jul 27, 2021
@darsnack
Copy link
Member

I don't think everything here should have been merged. The state of #44 was necessary for the downstream PRs.

@darsnack
Copy link
Member

The deleted types are also break MLDataPattern.jl. Can you revert this commit and just apply the version bump + test fix?

I agree with getting rid of types we don't need in the long term, but right now I am trying to update MLDataPattern.jl and MLLabelUtils.jl for the new interface. Too many clean up changes make refactoring the codebase too uncertain.

@CarloLucibello
Copy link
Member Author

ok

@darsnack
Copy link
Member

Thanks

@johnnychen94
Copy link
Member

My main focus is still on JuliaImages so sorry that I don't have more spare time to help here.

@CarloLucibello CarloLucibello mentioned this pull request Jul 27, 2021
darsnack added a commit to darsnack/LearnBase.jl that referenced this pull request Dec 29, 2021
This reverts commit aae4cbd, reversing
changes made to e08a6d7.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants