CLN deprecate save&load in favour of to_pickle&read_pickle
State: closed

Add `read_pickle` to top-level and `to_pickle` as instance methods, deprecation warning til 0.12 for save and load everwhere. See [lower down](https://github.com/pydata/pandas/pull/3787#issuecomment-19211203) for how it's working.

Both `read_pickle` and `to_pickle` are in `io.pickle`, save&load remain in `core.common` (but call to_pickle, read_pickle resp).

cc #3782.


Labels:

Comments:
though you were going to move to io/pickle.py ? I guess not much code....

need to add in io.rst / v0.11.1 the to_pickle/from_pickle refs....

also...the pickling section in the docs, I think in basics.rst should be moved to io.rst

I haven't managed to do that yet (getting some nasty circular import errors...)

I will move the pickle docs to io anyway.

you can leave save/load where they are (but import to_pickle/from_pickle from io/pickle) I think will fix
so core.common is not dependent on anything, and io/pickle can import it

I'm pretty sure I tried that exactly, but it wasn't working. Will stash apply and push it here so we can have a look.

@jreback something obvious?

Darn, this means I can't do my clever @set_doc thing... (Thanks it was indeed that!)

ok, updated (Still WIP).

Should I rename the method `from_pickle` from inside the generics class (in line with others). I wasn't sure if it should force it to be a DataFrame...

Related, while messing around... odd behaviour with the type checking of `from_csv`, should be a separate issue bit... huh? from_csv is surely not working correctly.

```
In [9]: s = pd.Series([1,2,3])

In [10]: s.to_csv('a')

In [11]: df = pd.DataFrame.from_csv('a')

In [12]: df
Out[12]:
            1
0
2013-06-01  2
2013-06-02  3
```

didn't realized there was a `from_pickle` in DF, yes get rid of it (well put a warning, then call read_pickle), we'll take it out in 0.12 (same with from_csv), but that's a bit more involved so its a separate issue (as its used internally)

as to your `from_csv` question, it shouldn't be attached to a particular class (and I think that might be the issue here, some of the default options are 'wrong') as they assume certain things

Nono, there isn't. I was going to call it ~~to_pickle~~ `read_pickle` inside the DataFrame, but lots* of other variables use the `from_` prefix:

```
In [2]: pd.DataFrame.fr
pd.DataFrame.from_csv      pd.DataFrame.from_items
pd.DataFrame.from_dict     pd.DataFrame.from_records
```

There's no `read_` counterparts inside the DataFrame object...

\* well, 4.

maybe I misundestaood....`read_pickle` is equiv of the `from_*`.....(of which `from_csv` will go way)...`from_dict` to stay thought IMHO it is useless clutter too 

`to_pickle` is replacing save....and fyi....just put in `core/generic` nothing added to `DataFrame` (put it in `PandasObject`) as `to_pickle`....

in 0.12...also fixing the inheritance so that things like `Index` will inherti from `PandasObject` (which is changed as well....its really the top-level object in 0.12).....but doesn't matter for now

Yeah that's where it is in the pr :), just giving example.

Currently `load` is also inside the DataFrame, but returns whatever the pickle was (i.e. could be a series). This is the bit I think is a little odd. (Am I making any sense?)

where is `load` inside Frame?

`pd.DataFrame.load/read_pickle` inherits from generic like you say... which does explain the behaviour. But I still kinda think it's confusing...

so take both read_pickle and load out of generic? I think that's reasonable.

Ruthless, but reasonable... :)

yes....read_xxx ONLY operate from the top level (which makes sense because you a) don't want to have to add to every conceivable object, and b) you could have different return objects based on your options (e.g. the squeeze option in pd.read_csv)

the `to_xxx` exist in core/generic only

so `load/save` you are leaving on core.common until 0.12 (with a warning)....

(and `save` with a warning in core/generic as well to be replace by `to_pickle`)

the idea was to completely presever back compat with the old names (but provide warnings)

then break on 0.12

@hayd 

as an aside.....there is a pandas.utils.decorators '@deprecate'...but of course I didn't use it anywhere.....

you want to change the warnings to use this? (i think also io.parsers.ExcelFile would need changing too)

Ok, your inline comments have confused me.

Just to confirm:
- include `read_pickle` in common, but _not_ in generic.
- include `to_pickle` in common & generic
- save/load as they are in generic & common (with warning these will be removed in 0.12)

_last two are already current behaviour of pr._

If that's right, I also should remove references to Series.read_pickle and friends from the api rsts.

(yeah I'll go through the warnings.)

- `read_pickle` should be _only_ in io/pickle.py (and picked up by the io api)
- `to_pickle` _only_ in `core/generic.py` (under `PandasObject` so it picks up `Series` (we are sort of breaking the ability to pickle explicity `Index`, but I don't think that matters)

save/load are screwed up......so
- `save`: remove from `core/common.py`,  with warnings in `core/generic.py`
- `load` as it exists in `core/common.py` but with a warning, remove from `core/generic.py`

end up with 1 save, 1 load (in the right places, just deprecated)

also remove `save/load` from top-level api (and instead temporarily import them from the io/api.py``
just for consistency (and put a note to remove in 0.12)

I didn't realize save/load were in so many places.....bad code :)

```
In [48]: s1.dump?
Type:       builtin_function_or_method
String Form:<built-in method dump of Series object at 0x10411d2a8>
Docstring:
a.dump(file)

Dump a pickle of the array to the specified file.
The array can be read back with pickle.load or numpy.load.

Parameters
----------
file : str
    A string naming the dump file.
```

_That comes from numpy... hmmm even if we delete load in pandas does this mean it'll still be there?_

I don't think that is actually necessary
can leave....doesn't really matter

i can't wait for msgpack...!

yep....if some is insterest in another project....I think that pandas needs to incorporate the msgpack cython code directly (I think the license is ok) to make it more efficient

@hayd looks good!

I need to tweak more with the docs to reflect save/delete changes we mentioned then I'll squish.

Just to _clarify_....I removed save, so no `pd.save` and no `DataFrame.load` (are we sure that's ok?).

_Ooh msgpack looks exciting :)._

`com.save` is there, is that imported into main api? (so `pd.save` should work and give you a warning,
like you have in common, but should still be callable as before)

`.load` looks ok to me (as it just gives you a warning then calls `read_pickle`)
I don't think its a big deal to eliminate `DataFrame.load` now....if you really want to put a warning on that ok I guess, but I think the original API/docs just used `pd.load` IIRC

Um, it's mentioned in the API reference, but will remove these mentions... :s

I will put back in pd.save... also tests are failing which use Series.from_pickle lol :)

almost ready to merge?