Skip to content
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.

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

place fields #126

Open
wants to merge 49 commits into
base: master
Choose a base branch
from
Open

place fields #126

wants to merge 49 commits into from

Conversation

MRScheid
Copy link
Contributor

Creating place field class

@bendichter
Copy link
Collaborator

bendichter commented Oct 29, 2020

@MRScheid looks good! Can you create something like these panels?

placefields

Taken from nelpy. We aren't using this library. Maybe we could, but might mean reinventing a lot at this point

@MRScheid
Copy link
Contributor Author

@MRScheid looks good! Can you create something like these panels?

placefields

Taken from nelpy. We aren't using this library. Maybe we could, but might mean reinventing a lot at this point

Yes, I'll give it a shot

@bendichter
Copy link
Collaborator

@MRScheid can you also make sure speed gating is used for the 1D rate maps?

@bendichter bendichter changed the title Initial code draft place fields Oct 30, 2020
nwbwidgets/placefield.py Outdated Show resolved Hide resolved
nwbwidgets/placefield.py Outdated Show resolved Hide resolved
nwbwidgets/placefield.py Outdated Show resolved Hide resolved
nwbwidgets/placefield.py Outdated Show resolved Hide resolved
nwbwidgets/view.py Outdated Show resolved Hide resolved
@bendichter
Copy link
Collaborator

@MRScheid could you allow the widget to optionally take as input a TimeSeries of velocity? Sometimes that is provided separately.

@MRScheid
Copy link
Contributor Author

MRScheid commented Nov 4, 2020

@MRScheid could you allow the widget to optionally take as input a TimeSeries of velocity? Sometimes that is provided separately.

I'm not sure I fully understand what you have in mind--would it be a TimeSeries of velocity in place of the position in the SpatialSeries? Would I then need to derive approximate position from the TimeSeries of velocity?

@MRScheid
Copy link
Contributor Author

MRScheid commented Nov 4, 2020

Also, what would be the file extension of the TimeSeries input? .nwb? Maybe this will be good to discuss during our meeting today

@bendichter
Copy link
Collaborator

Sometimes velocity is calculated separately with special smoothing function or something and I want to be able to use this in the widget if it is available. The TimeSeries would be an additional optional input that would be supplied in addition to SpatialSeries representing position. It would not be part of an independent file. If it is not supplied, the function would calculate speed as it does now.

firing_rate_ind += 1


fig, ax = plt.subplots()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fig, ax = plt.subplots()
fig, ax = plt.subplots(figsize=(7,7))

bendichter and others added 12 commits November 18, 2020 16:31
* figure size
…se from caching the calls in placefields.py with numpy arrays im the cache keys.

Incorporated an additional input in the widget to specify the width of the gaussian kernel independently for x and y respectively.

Built a wrapper function around the 2D place field calculation call so that the place field calculation function calls could be properly cached without the "unhashable key" errors.
Re-factor position and widget controls to make the PlaceFieldWidget class more easily extensible for the towers task place field widget
Refactored placefield to take separate pixel_widths. for x and y dimension.  Modified placefields to reflect this change.
Modifications:
- Disabled velocity button when not in use
- Modified both compute_2d_occupancy and compute_2d_n_spikes to recognize when the towers task place field widget is calling it so that speed is computed on just the x-dim.
- Modified compute_speed to work with 1-dim input
- Fixed the order of inputs to np.histogram2d.  Previously the x and y input were reversed from the order expected by the function.
- Refactored the pixel_width instance atrribute definition to be separate for x and y.  This allows the towers task place field widget extension to overwrite this method.
Made modifications to:
- NaN out any unexplored areas in the place field
- Add the ability to control the number of bins in the place field calculation.
@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #126 (c9fa416) into master (b9d0669) will decrease coverage by 3.15%.
The diff coverage is 16.08%.

❗ Current head c9fa416 differs from pull request most recent head 87a2433. Consider uploading reports for the commit 87a2433 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
- Coverage   63.25%   60.10%   -3.16%     
==========================================
  Files          47       49       +2     
  Lines        3013     3181     +168     
==========================================
+ Hits         1906     1912       +6     
- Misses       1107     1269     +162     
Flag Coverage Δ
unittests 60.10% <16.08%> (-3.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
nwbwidgets/view.py 94.11% <ø> (ø)
nwbwidgets/analysis/placefields.py 13.58% <13.58%> (ø)
nwbwidgets/placefield.py 16.32% <16.32%> (ø)
nwbwidgets/ecephys.py 52.94% <100.00%> (-3.63%) ⬇️
nwbwidgets/image.py 46.23% <0.00%> (-4.33%) ⬇️
nwbwidgets/utils/dynamictable.py 52.72% <0.00%> (-3.73%) ⬇️
nwbwidgets/ophys.py 55.20% <0.00%> (ø)
nwbwidgets/test/test_ecephys.py 100.00% <0.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9d0669...87a2433. Read the comment docs.

@MRScheid MRScheid marked this pull request as ready for review March 9, 2021 02:48
.gitignore Outdated
Comment on lines 132 to 134
nwbwidgets/controllers/group_and_sort_controllers_multi_select.py
nwbwidgets/placefield_group_and_sort.py
nwbwidgets/towers_task_placefield.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MRScheid what are these files? Why are you gitignoring them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bendichter I removed all these from the git ignore file--they were modules that I was developing with but weren't quite ready to be committed, I thought I'd get back around to them but I never did. They're irrelevant now.

Removed outdated files being ignored
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants