sample behaviour

[LukasHedegaard]

Currently, if more samples are requested on .sample, than are available in the dataset, we will sample some samples multiple times. Should we raise an error instead?

[iliiliiliili]

We can add no_more_samples parameter to be able to:

1. 'error' – raise Exception if no more samples.
2. 'None' – out of bounds samples will be None. (Can it be useful?)
3. 'cycle' – start from beginning
4. 'repeat' – repeat last sample

[LukasHedegaard]

I don't think the 'None' version makes sense. Also, sampling is random, so I don't think a distinction between 'cycle' and 'repeat' makes sense.
Then it's either throw an error or sample additional randomly.
Does it make sense for the user to need to determine this by argument?
A middle-ground solution could be to emit a warning when "oversampling". What do you think?

[iliiliiliili]

Maybe we can have allow_oversampling=False so it raises Exception when trying to oversample, but if user sets it to True, we will just continue sampling randomly.

[clegaard]

My question is, does it really make sense to sample outside of len-1 index?
If the user wants to sample more than this could they not use the repeat operation?

[LukasHedegaard]

Scenarios with oversampling will probably be due to an error in user-logic, and I think it’s better to get an error earlier rather than later.

Should a creative user finds the need for it, he could always do:

ds_sampled = ds.sample(42)
ds_oversampled = ds.concat(ds_sampled)

Though not that bad, it does break the call-chain. And seeing as the implementation is trivial, why not add the allow_oversampling flag as @iliiliiliili proposed, with a default arg (False) that optimises for the more frequent scenario (possible error in user logic)?

[clegaard]

Should this also extend to take function as well? If one is allowed I would expect the other to be as well.
I definitely prefer the flag to do funky stuff like the concat.

However, if the user wants to sample more samples than what is present they could use the repeat function? This would be more transparent in my opinion and would not add any complexity.

ds_sampled = ds.repeat(2).sample(42)
ds_taken = ds.repeat(2).take(42)

[LukasHedegaard]

Let's remove the oversampling and throw an error instead