FluxML / FastAI.jl

Repository of best practices for deep learning in Julia, inspired by fastai
https://fluxml.ai/FastAI.jl
MIT License
585 stars 51 forks source link

`TaskDataset` does not sub-type `MLUtils.AbstractDataContainer` #252

Open darsnack opened 2 years ago

darsnack commented 2 years ago

I would sub-type MLUtils.AbstractDataContainer to get some sensible defaults like Base.iterate defined on top of the MLUtils.jl interface.

lorenzoh commented 2 years ago

That’s a good idea!

dfarmer commented 1 year ago

I like the package and was hoping to contribute. This one seems like a pretty easy place to start. As far as code goes I guess it's trivial? (I'm new to Julia, though not to programming so maybe I'm wrong here).

struct TaskDataset{TData, TTask <: LearningTask, TContext <: Context} <: MLUtils.AbstractDataContainer
    data::TData
    task::TTask
    context::TContext
end

But I was trying to figure out what tests I can add. I see MLUtils defines Base.size, Base.iterate, and Base.lastindex but I'm having a little trouble in FastAI.jl figuring out the right place to put the tests (seems like most of them are in the implementation file, but I didn't see any in taskdata.jl) and I was having a little trouble figuring out a quick and easy way to instantiate a test TaskDataset. If you're busy then no worries I'll just keep poking around until I figure it out but I'd glady take any tips too.

lorenzoh commented 1 year ago

Yep, it should be a simple change. Happy to see contribution!