Skip to content

Fix cache roundtrips truncating dataframes #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 17, 2018
Merged

Fix cache roundtrips truncating dataframes #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a9b3d64
Replace deprecated from_csv calls with read_csv
JFPerkins Jul 30, 2018
f0fba88
Remove unused from_csv references in tests
JFPerkins Jul 30, 2018
05f7ac8
Add test to cover actual use of cache that prompted #207
JFPerkins Jul 30, 2018
23b8dc6
Remove reference to from_csv from documentation
JFPerkins Jul 30, 2018
8ad1afb
restrict pytest version pending investigation into why test discovery…
JFPerkins Jul 31, 2018
de20bed
Update 207 from master
JFPerkins Oct 12, 2018
60ed302
Don't set max version for pytest
JFPerkins Oct 12, 2018
f54438e
Mouse connectivity round tripping cacheing test
JFPerkins Oct 12, 2018
deca125
Fix syntax error in example
JFPerkins Oct 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions allensdk/api/cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ def load_csv(self,
index : string, optional
post-rename column to use as the row label.
'''
data = pd.DataFrame.from_csv(path)
data = pd.read_csv(path, parse_dates=True)

Cache.rename_columns(data, rename)

Expand Down Expand Up @@ -383,14 +383,14 @@ def csv_writer(pth, gen):
def cache_csv_json():
return {
'writer': Cache.csv_writer,
'reader': lambda f: pd.DataFrame.from_csv(f).to_dict('records')
'reader': lambda f: pd.read_csv(f, parse_dates=True).to_dict('records')
}

@staticmethod
def cache_csv_dataframe():
return {
'writer': Cache.csv_writer,
'reader' : pd.DataFrame.from_csv
'reader' : lambda f: pd.read_csv(f, parse_dates=True)
}

@staticmethod
Expand Down Expand Up @@ -422,7 +422,7 @@ def cache_json():
def cache_csv():
return {
'writer': Cache.csv_writer,
'reader': pd.DataFrame.from_csv
'reader': lambda f: pd.read_csv(f, parse_dates=True)
}

@staticmethod
Expand Down Expand Up @@ -525,7 +525,7 @@ def wrap(self, fn, path, cache,
else:
data = ju.read(path)
elif return_dataframe is True:
data = pd.DataFrame.from_csv(path)
data = pd.read_csv(path, parse_dates=True)
else:
raise ValueError(
'save_as_json=False cannot be used with return_dataframe=False')
Expand Down
2 changes: 1 addition & 1 deletion allensdk/core/mouse_connectivity_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ def get_experiment_structure_unionizes(self, experiment_id,
pre=col_rn,
post=filter_fn,
writer=lambda p, x : pd.DataFrame(x).to_csv(p),
reader=pd.DataFrame.from_csv)
reader=lambda x: pd.read_csv(x, index_col=0, parse_dates=True))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check to make sure that the file saved here puts the correct column first when written?

cc @NileGraddis

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a test that covers it. I was going by the documentation for to_csv (https://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.to_csv.html) which indicates that by default it is writing an index label column.

In general I'm planning to add more tests like the one I added for get_ephys_features that actually round trip the files, as I noticed all the tests we have seem to mock out the file writing.


def rank_structures(self, experiment_ids, is_injection, structure_ids=None, hemisphere_ids=None,
rank_on='normalized_projection_volume', n=5, threshold=10**-2):
Expand Down
58 changes: 29 additions & 29 deletions allensdk/test/api/test_cacheable.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@

_msg = [{'whatever': True}]
_pd_msg = pd.DataFrame(_msg)
_csv_msg = pd.DataFrame.from_csv(StringIO.StringIO(""",whatever
_csv_msg = pd.read_csv(StringIO.StringIO(""",whatever
0,True
"""))
"""), index_col=0)


@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch('csv.DictWriter')
@patch.object(pd.DataFrame, 'from_csv', return_value=_csv_msg)
def test_cacheable_csv_dataframe(from_csv, dictwriter, ju_read_url_get,
@patch('pandas.read_csv', return_value=_csv_msg)
def test_cacheable_csv_dataframe(read_csv, dictwriter, ju_read_url_get,
ju_read, ju_write):
@cacheable()
def get_hemispheres():
Expand All @@ -79,7 +79,7 @@ def get_hemispheres():

ju_read_url_get.assert_called_once_with(
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
from_csv.assert_called_once_with('/xyz/abc/example.txt')
read_csv.assert_called_once_with('/xyz/abc/example.txt', parse_dates=True)
assert not ju_write.called, 'write should not have been called'
assert not ju_read.called, 'read should not have been called'
mkdir.assert_called_once_with('/xyz/abc')
Expand All @@ -90,8 +90,8 @@ def get_hemispheres():
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch.object(Manifest, 'safe_mkdir')
@patch.object(pd.DataFrame, 'from_csv', return_value=_csv_msg)
def test_cacheable_json(from_csv, mkdir, ju_read_url_get, ju_read, ju_write):
@patch('pandas.read_csv', return_value=_csv_msg)
def test_cacheable_json(read_csv, mkdir, ju_read_url_get, ju_read, ju_write):
@cacheable()
def get_hemispheres():
return RmaApi().model_query(model='Hemisphere')
Expand All @@ -104,7 +104,7 @@ def get_hemispheres():

ju_read_url_get.assert_called_once_with(
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
assert not from_csv.called, 'from_csv should not have been called'
assert not read_csv.called, 'read_csv should not have been called'
ju_write.assert_called_once_with('/xyz/abc/example.json',
_msg)
ju_read.assert_called_once_with('/xyz/abc/example.json')
Expand Down Expand Up @@ -136,8 +136,8 @@ def get_hemispheres_excpt():
@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch.object(pd.DataFrame, 'from_csv', return_value=_csv_msg)
def test_cacheable_no_cache_csv(from_csv, ju_read_url_get, ju_read, ju_write):
@patch('pandas.read_csv', return_value=_csv_msg)
def test_cacheable_no_cache_csv(read_csv, ju_read_url_get, ju_read, ju_write):
@cacheable()
def get_hemispheres():
return RmaApi().model_query(model='Hemisphere')
Expand All @@ -149,19 +149,19 @@ def get_hemispheres():
assert df.loc[:, 'whatever'][0]

assert not ju_read_url_get.called
from_csv.assert_called_once_with('/xyz/abc/example.csv')
read_csv.assert_called_once_with('/xyz/abc/example.csv', parse_dates=True)
assert not ju_write.called, 'json write should not have been called'
assert not ju_read.called, 'json read should not have been called'


@patch("pandas.io.json.read_json", return_value=_pd_msg)
@patch.object(pd.DataFrame, "from_csv", return_value=_csv_msg)
@patch("pandas.read_csv", return_value=_csv_msg)
@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch.object(Manifest, 'safe_mkdir')
def test_cacheable_json_dataframe(mkdir, ju_read_url_get, ju_read, ju_write,
from_csv, mock_read_json):
read_csv, mock_read_json):
@cacheable()
def get_hemispheres():
return RmaApi().model_query(model='Hemisphere')
Expand All @@ -174,7 +174,7 @@ def get_hemispheres():

ju_read_url_get.assert_called_once_with(
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
assert not from_csv.called, 'from_csv should not have been called'
assert not read_csv.called, 'read_csv should not have been called'
mock_read_json.assert_called_once_with('/xyz/abc/example.json',
orient='records')
ju_write.assert_called_once_with('/xyz/abc/example.json', _msg)
Expand All @@ -183,14 +183,14 @@ def get_hemispheres():


@patch("pandas.io.json.read_json", return_value=_pd_msg)
@patch.object(pd.DataFrame, "from_csv", return_value=_csv_msg)
@patch("pandas.read_csv", return_value=_csv_msg)
@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch('csv.DictWriter')
@patch.object(Manifest, 'safe_mkdir')
def test_cacheable_csv_json(mkdir, dictwriter, ju_read_url_get, ju_read,
ju_write, from_csv, mock_read_json):
ju_write, read_csv, mock_read_json):
@cacheable()
def get_hemispheres():
return RmaApi().model_query(model='Hemisphere')
Expand All @@ -207,7 +207,7 @@ def get_hemispheres():

ju_read_url_get.assert_called_once_with(
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
from_csv.assert_called_once_with('/xyz/example.csv')
read_csv.assert_called_once_with('/xyz/example.csv', parse_dates=True)
dictwriter.return_value.writerow.assert_called()
assert not mock_read_json.called, 'pj.read_json should not have been called'
assert not ju_write.called, 'ju.write should not have been called'
Expand All @@ -219,9 +219,9 @@ def get_hemispheres():
@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch.object(pd.DataFrame, "from_csv")
@patch("pandas.read_csv")
@patch.object(pd.DataFrame, "to_csv")
def test_cacheable_no_save(to_csv, from_csv, ju_read_url_get, ju_read,
def test_cacheable_no_save(to_csv, read_csv, ju_read_url_get, ju_read,
ju_write):
@cacheable()
def get_hemispheres():
Expand All @@ -234,17 +234,17 @@ def get_hemispheres():
ju_read_url_get.assert_called_once_with(
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
assert not to_csv.called, 'to_csv should not have been called'
assert not from_csv.called, 'from_csv should not have been called'
assert not read_csv.called, 'read_csv should not have been called'
assert not ju_write.called, 'json write should not have been called'
assert not ju_read.called, 'json read should not have been called'


@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch.object(pd.DataFrame, "from_csv", return_value=_csv_msg)
@patch("pandas.read_csv", return_value=_csv_msg)
@patch.object(pd.DataFrame, "to_csv")
def test_cacheable_no_save_dataframe(to_csv, from_csv, ju_read_url_get,
def test_cacheable_no_save_dataframe(to_csv, read_csv, ju_read_url_get,
ju_read, ju_write):
@cacheable()
def get_hemispheres():
Expand All @@ -257,19 +257,19 @@ def get_hemispheres():
ju_read_url_get.assert_called_once_with(
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
assert not to_csv.called, 'to_csv should not have been called'
assert not from_csv.called, 'from_csv should not have been called'
assert not read_csv.called, 'read_csv should not have been called'
assert not ju_write.called, 'json write should not have been called'
assert not ju_read.called, 'json read should not have been called'


@patch.object(pd.DataFrame, "from_csv", return_value=_csv_msg)
@patch("pandas.read_csv", return_value=_csv_msg)
@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch('csv.DictWriter')
@patch.object(Manifest, 'safe_mkdir')
def test_cacheable_lazy_csv_no_file(mkdir, dictwriter, ju_read_url_get,
ju_read, ju_write, from_csv):
ju_read, ju_write, read_csv):
@cacheable()
def get_hemispheres():
return RmaApi().model_query(model='Hemisphere')
Expand All @@ -289,16 +289,16 @@ def get_hemispheres():
'http://api.brain-map.org/api/v2/data/query.json?q=model::Hemisphere')
open_mock.assert_called_once_with('/xyz/abc/example.csv', 'w')
dictwriter.return_value.writerow.assert_called()
from_csv.assert_called_once_with('/xyz/abc/example.csv')
read_csv.assert_called_once_with('/xyz/abc/example.csv', parse_dates=True)
assert not ju_write.called, 'json write should not have been called'
assert not ju_read.called, 'json read should not have been called'


@patch("allensdk.core.json_utilities.write")
@patch("allensdk.core.json_utilities.read", return_value=_msg)
@patch("allensdk.core.json_utilities.read_url_get", return_value={'msg': _msg})
@patch.object(pd.DataFrame, "from_csv", return_value=_csv_msg)
def test_cacheable_lazy_csv_file_exists(from_csv, ju_read_url_get, ju_read,
@patch("pandas.read_csv", return_value=_csv_msg)
def test_cacheable_lazy_csv_file_exists(read_csv, ju_read_url_get, ju_read,
ju_write):
@cacheable()
def get_hemispheres():
Expand All @@ -312,6 +312,6 @@ def get_hemispheres():
assert df.loc[:, 'whatever'][0]

assert not ju_read_url_get.called
from_csv.assert_called_once_with('/xyz/abc/example.csv')
read_csv.assert_called_once_with('/xyz/abc/example.csv', parse_dates=True)
assert not ju_write.called, 'json write should not have been called'
assert not ju_read.called, 'json read should not have been called'
10 changes: 5 additions & 5 deletions allensdk/test/api/test_pager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ def pager():

_msg = [{'whatever': True}]
_pd_msg = pd.DataFrame(_msg)
_csv_msg = pd.DataFrame.from_csv(StringIO.StringIO(""",whatever
_csv_msg = pd.read_csv(StringIO.StringIO(""",whatever
0,True
"""))
"""), index_col=0)

_read_url_get_msg5 = [{'msg': _msg},
{'msg': _msg},
Expand Down Expand Up @@ -145,11 +145,11 @@ def get_genes(**kwargs):
(Cache.cache_csv,
Cache.cache_csv_json,
Cache.cache_csv_dataframe))
@patch.object(pd.DataFrame, "from_csv", return_value=_csv_msg)
@patch("pandas.read_csv", return_value=_csv_msg)
@patch("allensdk.core.json_utilities.read_url_get",
side_effect=_read_url_get_msg5)
@patch("os.makedirs")
def test_cacheable_pageable_csv(os_makedirs, ju_read_url_get, from_csv,
def test_cacheable_pageable_csv(os_makedirs, ju_read_url_get, read_csv,
cache_style):
archive_templates = \
{"cam_cell_queries": [
Expand Down Expand Up @@ -193,7 +193,7 @@ def get_cam_cell_metrics(*args,
[0, 1, 2, 3, 4, 5])

assert ju_read_url_get.call_args_list == list(expected_calls)
from_csv.assert_called_once_with('/path/to/cam_cell_metrics.csv')
read_csv.assert_called_once_with('/path/to/cam_cell_metrics.csv', parse_dates=True)

assert csv_writerow.call_args_list == [call({'whatever': 'whatever'}),
call({'whatever': True}),
Expand Down
Loading