search

AllenInstitute / visual_behavior_analysis

Python package for analyzing behavioral data for Brain Observatory: Visual Behavior
Other
21 stars 6 forks source link

circular import in data_access.reformat #730

Closed dougollerenshaw closed 3 years ago

dougollerenshaw commented 3 years ago

data_access.loading imports data_access.reformat: https://github.com/AllenInstitute/visual_behavior_analysis/blob/8205d115078b6302475e6d3847eaf94f6fb5765c/visual_behavior/data_access/loading.py#L10

and data_access.reformat imports data_access.loading https://github.com/AllenInstitute/visual_behavior_analysis/blob/8205d115078b6302475e6d3847eaf94f6fb5765c/visual_behavior/data_access/reformat.py#L5

We should eliminate this circular import.

Inside of reformat.py, loading.py is used only in the add_image_set_exposure_number_to_experiments_table function: https://github.com/AllenInstitute/visual_behavior_analysis/blob/8205d115078b6302475e6d3847eaf94f6fb5765c/visual_behavior/data_access/reformat.py#L73

@DowntonCrabby and/or @matchings: is this function still necessary or does this functionality now exist in the AllenSDK?

matchings commented 3 years ago

This does exist in the SDK now, but i believe it is only for data that is released. I think we still need this functionality in VBA for newly collected data. Could we put it in a different place to avoid circular import? Like in data_access.processing.py? Or even in loading.py directly

dougollerenshaw commented 3 years ago

It looks like we can pass the behavior_session_table from loading when reformat_experiments_table is called here:

https://github.com/AllenInstitute/visual_behavior_analysis/blob/8205d115078b6302475e6d3847eaf94f6fb5765c/visual_behavior/data_access/loading.py#L188

That would allow us to avoid importing loading from within reformat.

dougollerenshaw commented 3 years ago

@matchings see this commit: https://github.com/AllenInstitute/visual_behavior_analysis/pull/731/commits/4f75fa67fc9304c58f3900800d4de09b33f7f0ff

I'm including this as part of the PR to get VBA compatible with SDK 2.10.2

dougollerenshaw commented 3 years ago

That wasn't the only place that loading.py was used inside of reformat.py. But I'm following the same strategy of passing the necessary tables into the functions in reformat.py

dougollerenshaw commented 3 years ago

Fixed in PR #731