Preserve split type when realoding dataset
State: closed | Author: mariosasko (COLLABORATOR)

Fixes #2167 

Using `eval` is not ideal for security reasons (in web apps I assume), but without it the code would be much more complex IMO.

In terms of style, instead of explicitly importing a private member (`_RelativeInstruction`), we can add these imports at the top of the module:
```python
from . import arrow_reader  # gives us access to ReadInstruction and _RelativeInstruction
from . import splits # gives us access to NamedSplit
```

and then define the `eval` globals as follows:
```python
{**arrow_reader.__dict__, **splits.__dict__}
```




Comments: Thanks for diving into this !

Before going further, I just want to make sure if using `eval` is the right solution
Personally I'm not a big fan of `eval` since it has many security concerns. Also storing string representations of python objects in the json files is not ideal either IMO, so maybe it's possible to change this aspect instead.

Maybe it would be better to convert the `_RelativeInstruction` to a string (or "specs") ?
It looks like `ReadInstruction.from_spec` already exists, but not the other way around.
The specs are the string representation of instructions. For example: `train+validation[:50%]`.

Let me know what you think ! And thanks again, this issue has been here for a while now ^^

@lhoestq Yes, before going with `eval`, I thought about this approach with the "spec". The only issue with this approach is that we have to come up with a represenation for the `rounding` arg.

What do you think about this (maybe too verbose)?
```python
>>> print(ReadInstruction("train", rounding="pct1_dropremainder", from_=10, to=30).to_spec())
train[10:30](pct1_dropremainder)

Good idea !

First we must note that the rounding is only used for percentage instructions.
For absolute instructions there's no rounding ambiguity.

By default the rounding is set to `closest`. For example if you have a train set of 999 examples and if you provide an instruction spec `"train[:1%]"`, you're going to get the first ten examples (while the `pct1_dropremainder ` rounding would return 9 examples).

Currently there's no way to get an instruction with a `pct1_dropremainder` rounding strategy from an instruction spec.
So we can either drop the support of `pct1_dropremainder` or define a way to use this strategy from a spec.
I don't think dropping `pct1_dropremainder` would be a good idea since it allows to load each percent to all have the same number of examples (even the last one). Therefore I think your suggestion makes total sense and we should add a representation of this rounding strategy.

I like what you suggested `train[10%:30%](pct1_dropremainder)` is fine, and it seems compatible with the regex that parses the instructions specs.

@lhoestq I've made the changes as you suggested. Ready for the review.

@lhoestq I've added a test and addressed the comments.

Additionally, `ReadInstruction` is converted to its spec form in `builder.py` to avoid a circular import that would happen if this logic was in `arrow_reader.py`.  If you think it's better to have this logic in `arrow_reader.py`, the import can be delayed by putting it inside a function.