neuroinformatics-unit / rsp-vision

Analysis and visualization of calcium imaging data from retrosplenial cortex
BSD 3-Clause "New" or "Revised" License
8 stars 0 forks source link

Improve analysis #49

Closed lauraporta closed 1 year ago

lauraporta commented 1 year ago

@openai: ignore ☝🏻stops future reviews of the bot

👉🏻 @ Future human reviewer

I am experimenting using a chatgpt bot to have PR reviewed, while waiting for copilot X to arrive.

The bot is not able to process files that are too large, and unfortunately those are the core of the PR. These files are:

So no summary and no comments on the code are provided for these two files. Therefore I have updated the descriptions of the bot filling them up with the missing info.

I noticed also that applies comments relevant to one file in the wrong file. I'll see if I can move them around. Sorry about that.

⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️ N.B.: a previous PR (#25) had many relevant suggestions that are still applicable to this one. They are not addressed in this PR since they require extensive refactoring. This PR is mostly focused on adding new functionalities and writing better tests. ⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️

Finally, these changes where actually done while developing the dashboard, which you can find in the commits. Please ignore any changes related to it. I have split the changes related to the analysis and put them all in this PR to keep things cleaner.

Laura's summary

Broad description of the main changes:

Summary by OpenAI

Release Notes:

Changes abound, code quality improved, Bugs squashed, docs clarified, tests approved, With new features added, and refactorings too, This PR brings joy, and we thank you! 🎉👏

github-actions[bot] commented 1 year ago

:robot: OpenAI

High-level Summary

The changes made to the codebase include improvements in logic, security, performance, data races, consistency, error handling, maintainability, modularity, complexity, and optimization. The changeset includes modifications to various files such as .gitignore, MANIFEST.in, demo_cli.py, app.py, config_switches.py, load_data.py, data_raw.py, parser2p.py, parser2pRSP.py, and test files. The changes involve adding new parameters to the config file, refactoring functions, improving documentation, adding unit tests, and modifying the application's structure to improve its usability.

Files Summary

File Changes
.gitignore, setup_for_demo.py Added .pickle and .obsidian to .gitignore file. Added fitting parameters to the config file and modified some existing parameters such as fps_tree_photon, spatial_frequencies, temporal_frequencies, directions, n_spatial_frequencies, n_temporal_frequencies, and n_directions.
MANIFEST.in Added two lines to include documentation files in the distribution package. Removed a line that includes YAML files and does not exclude .md and .png files from being included.
demo_cli.py, rsp_vision/main.py Added import statement and calls analysis_pipeline() function if it exists. The main function has been removed and replaced with a new app.py file that allows the application to be started by running python3 demo_cly.py. The analysis class is now called FrequencyResponsiveness and used with the name responsiveness for short when instantiated. The PhotonData object has been changed to accommodate these changes.
docs/reproduce_matlab_output.md Added proof of accurate MATLAB output reproduction and comparison with Python implementation. Handles NaN values better in the new implementation.
rsp_vision/console_application/app.py Added a decorator to handle exceptions, a function to start logging, and a function to get the name of the module for logging purposes. The main function now uses rich.prompt to get input from the user instead of command-line arguments. Imported new modules and classes, such as logging, pickle, rich, and PhotonData.
rsp_vision/load/config_switches.py, rsp_vision/load/load_data.py Refactored get_fps function to include type hints and remove unused ascii_array_to_string function. The read function is removed and replaced with read_config_file. The yaml module is imported. The load_data function now calls a new function load_data_from_filename which replaces the old load function. A new function read_config_file is added to read the configuration file.
rsp_vision/objects/data_raw.py, rsp_vision/objects/file.py, rsp_vision/objects/folder_naming_specs.py Improved the documentation of classes and their methods. It adds more details to the docstrings, clarifies some comments, and fixes typos.
rsp_vision/objects/parsers2p/parser2p.py, rsp_vision/objects/parsers2p/parser2pRSP.py The changes in parser2p.py involve minor modifications to the docstrings and comments. The changes improve readability and do not introduce new functionality or modify existing logic. The diff updates the docstrings and formatting of Parser2pRSP class in parser2pRSP.py. It also updates the comments and formatting of some methods.
tests/test_integration/calculate_outputs.py, tests/test_integration/generate_mock_data.py, tests/test_integration/test_photon_data.py, tests/test_integration/test_sf_tf.py, tests/test_integration/test_photon_data.py, tests/test_integration/test_sf_tf_static_methods.py Minor changes to add fixtures, rename variables, and use fixtures instead of functions for testing purposes. No issues with logic, security, performance, data races, consistency, error handling, maintainability, modularity, complexity, or optimization were found.
tests/test_unit/test_gaussian_related_methods.py Added unit tests for Gaussian-related methods in gaussians_calculations.py. Tests include functions to fit a 2D Gaussian to data, create a Gaussian matrix, and perform a single Gaussian fit.

Chat with 🤖 OpenAI Bot (@openai)

Code suggestions


Files ignored due to filter (1) ### Ignored files * tox.ini
Files not summarized due to errors (5) ### Failed to summarize * rsp_vision/analysis/gaussians_calculations.py (diff tokens exceeds limit) * rsp_vision/analysis/spatial_freq_temporal_freq.py (diff tokens exceeds limit) * rsp_vision/objects/photon_data.py (diff tokens exceeds limit) * tests/test_integration/generate_mock_data.py (diff tokens exceeds limit) * tests/test_integration/test_sf_tf.py (diff tokens exceeds limit)
Files not reviewed due to simple changes (23) ### Skipped review * .gitignore * MANIFEST.in * demo_cli.py * docs/reproduce_matlab_output.md * rsp_vision/console_application/app.py * rsp_vision/load/config_switches.py * rsp_vision/load/load_data.py * rsp_vision/load/read_config.py * rsp_vision/main.py * rsp_vision/objects/data_raw.py * rsp_vision/objects/file.py * rsp_vision/objects/folder_naming_specs.py * rsp_vision/objects/parsers2p/parser2p.py * rsp_vision/objects/parsers2p/parser2pRSP.py * rsp_vision/plots/plotter.py * rsp_vision/utils.py * setup_for_demo.py * tests/conftest.py * tests/test_integration/calculate_outputs.py * tests/test_integration/test_photon_data.py * tests/test_integration/test_sf_tf_static_methods.py * tests/test_unit/test_gaussian_related_methods.py * tests/test_unit/test_utils.py
lauraporta commented 1 year ago

Joe, thanks for all your comments and suggestions. Right now I would not address them, I add them to the backlog of the possible improvements that I could do in the future.