search

recommenders / rival

RiVal recommender system evaluation toolkit
rival.recommenders.net
Apache License 2.0
150 stars 40 forks source link

Generalized code for CrossValidationSplitter #94

Closed aleSuglia closed 9 years ago

aleSuglia commented 9 years ago

I need to change the CrossValidationSplitter code in order to match my DataModel type parameters, so I've decided to generalize the whole code. All the tests pass as expected.

P.S. I've noticed that the other splitter classes have some kind of restrictions so I wasn't able to generalize them. For example, in the RandomSplitter you create some strings and then you convert them to Long. This kind of stuff, which depends on the specific type that you use, are really difficult to generalize. I'll report another issue in which I'll explain better this aspect.

abellogin commented 9 years ago

Thank you, Alessandro. Since we are preparing the 0.2 release, I think we should wait until next release, so we can also generalize the other Splitters. Do you agree, @alansaid?

alansaid commented 9 years ago

I agree @abellogin. I propose a feature freeze until 0.2 is released (it's in my todo-list for tomorrow), and we push this to 0.3.

alansaid commented 9 years ago

@aleSuglia can you please create an issue which describes what it is that's being fixed here?

This is mainly for helping us keep a record of what's been done where.

aleSuglia commented 9 years ago

Sure. I'll do it as soon as possible. On 7 Aug 2015 14:14, "alansaid" notifications@github.com wrote:

@aleSuglia https://github.com/aleSuglia can you please create an issue which describes what it is that's being fixed here?

This is mainly for helping us keep a record of what's been done where.

— Reply to this email directly or view it on GitHub https://github.com/recommenders/rival/pull/94#issuecomment-128685243.

alansaid commented 9 years ago

I'm going to merge this into master as this definitely is something we want to pursue and from the looks of it it does what's intended. Thanks for kicking this off @aleSuglia.