ENH: make to_json & to_csv transformers have deterministic filenames #862
Conversation
| """ | ||
| Write the data model to a .json file and return a url based data model. | ||
| """ | ||
| data_json = _data_to_json_string(data) |
There was a problem hiding this comment.
I tested the logic locally and it works as expected. In particular I made multiple plots of the same dataset (one file) and then mutated the data frame (second file). Very nice!
altair/utils/data.py
Outdated
| def _data_to_csv_string(data): | ||
| """return a CSV string representation of the input data""" | ||
| check_data_type(data) | ||
| if isinstance(data, pd.DataFrame): |
There was a problem hiding this comment.
Just thinking - shouldn't the csv transformer also work with dict/values as the json transformer does? I know the original code didn't work this way, but I don't see any reason to not add the same logic here.
There was a problem hiding this comment.
Sounds good – I'll add that.
altair/utils/data.py
Outdated
| data = sanitize_dataframe(data) | ||
| return data.to_csv(index=False) | ||
| else: | ||
| raise NotImplementedError('to_csv only works with Pandas DataFrame objects.') |
There was a problem hiding this comment.
And this error message would need to change...
altair/utils/tests/test_data.py
Outdated
| @@ -63,3 +66,39 @@ def test_type_error(): | |||
| for f in (sample, limit_rows, to_values): | |||
| with pytest.raises(TypeError): | |||
| pipe(0, f) | |||
|
|
|||
|
|
|||
| def test_to_json(): | |||
There was a problem hiding this comment.
Maybe also a simple test that covers the dict/values path?
|
I added support for dict input in to_csv, and also added tests of dict input for to_json and to_csv. |
|
Great, thanks! |
Addresses #857
The idea here is that when someone uses the
jsonorcsvdata transformer, the dataframes are stored on disk with a filename that is determined from the hash of the data contents. This prevents the proliferation of temporary files when doing a lot of plotting.