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.

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

Jump to bottom

RawPosition cannot handle multiple pynwb position objects #613

Closed
dpeg22 opened this issue Aug 2, 2023 · 1 comment · Fixed by #628
Closed

RawPosition cannot handle multiple pynwb position objects #613

dpeg22 opened this issue Aug 2, 2023 · 1 comment · Fixed by #628
Assignees
@CBroz1
Labels
position

Comments

@dpeg22
Copy link
Collaborator

dpeg22 commented Aug 2, 2023 •
edited
Loading

Describe the bug
RawPosition currently cannot handle multiple pynwb Spatial Series objects. The Spatial Series objects currently referenced by entries in RawPosition contain ≥4 columns (time, xloc1, yloc1, xloc2, yloc2) to reflect the locations of the front and back LEDs on the animal's LED ring. NWB versions >2.5 require Spatial Series objects have a second dimension with length ≤3 as detailed in LorenFrankLab/rec_to_nwb#45. To comply with this requirement, we should break position tracking into >1 Spatial Series object.
RawPosition has a single column for raw_position_object_id to store the reference to the associated position object in the NWB file, thus allowing for only a single associated NWB object.

Some initial ideas on how we could allow for >1 associated position object:

  • alter RawPosition to add another column to hold a second object id
  • we could add a column that would hold a list of all raw position object ids
  • potentially add a part table that would have an entry for each associated object id and be collated when fetch_nwb is called
@lfrank
Copy link
Contributor

lfrank commented Aug 2, 2023

My first thought is that the Part table option might be best, as it would allow for an arbitrary number of spatial series in the future.

CBroz1 added a commit to CBroz1/spyglass that referenced this issue Aug 18, 2023
@CBroz1
Refactor trodes position LorenFrankLab#613
bb7580c
@CBroz1 CBroz1 mentioned this issue Aug 21, 2023
Merged
@CBroz1 CBroz1 linked a pull request Aug 21, 2023 that will close this issue
#613 - RawPosition handle multiple pynwb position objects #628
Merged
@edeno edeno added the position label Aug 22, 2023
@CBroz1 CBroz1 mentioned this issue Sep 13, 2023
Closed
edeno pushed a commit that referenced this issue Oct 10, 2023
@CBroz1
#613 - RawPosition handle multiple pynwb position objects (#628)
ce0bca4 
* Selective fetch from cbroz/master

* Fetch additional file from cbroz1/master to pass CI/CD

* Add RawPos fetch method implementations. Object -> PosObject

* Refactor PosIntervalMap helpers

* Revert typo

* Bugfixes for ripple

* Blackify

* WIP: minor edits

* Add restriction to fetch1_dataframe

* Set pos id default for migration. Rename Trodes params

* Fix typos

* Refactor position helpers

* Minor position_trodes fixes
edeno pushed a commit that referenced this issue Oct 10, 2023
@CBroz1
More notebook revisions (#616)
99d8f00 
* Restructure for mkdocs

* Minor docstring edits for mkdocs

* Adjust installer docs

* Permit json config

* Update docstrings for mkdocs. Add images

* Minor fixes. Rename publish action. Add changelog notes

* hard wrap changes, markdownlint

* blackify

* Fix version cmd. Minor doc wording

* mkdocstrings require empty inits to identify submodules

* 🤦‍♂️ Publish docs CI/CD `main`->`master`

* Edit `get_part`; Add `merge_fetch`

* Spelling fixes

* Refactor restrict_parts. Adjust mkdocs nav.

* Docs adjust for Merge tables

* Fix `merge_get_part`

* Use hatch for docs version

* Update changelog

* Typo

* Spellcheck config

* WIP: dict/str ristrict consistency

* Normalize restriction/classmeth. Add notes on why

* Typos

* Docs publish on tag

* Edit changelog: Add links, patch version bump

* 🧪 Test gh-actions debug

* get-part multi-source flag

* Add mutual exclusivity flag from pos branch

* See details. Notebook work, config overhaul

gitignore: add exclude example config
mkdocs: new notebook names
notebooks: complete revamp for minirec data and more links to docs
init: add new load_config, isort imports
common_lab:
	- adjust to accept names in Last, First format (nwb-compliant)
	- continue to use First Last name structure in database - yes?
common_nwb: use new load_config, change `assert` to `raise`
insert_sessions: permit paths, use file name, use raw dir
storage_dirs: remove redundant funcs for base_dir
settings: implement new base_dir system
	- allows base/raw/etc to be independent
	- defaults to dj.config, then env vars, then sets default rel paths

* WIP: fix failing tests related to base_dir edits

* underscore-prefix Merge. Linter fixes

* See Details. Notebook overhaul

- dj_config: accept base dir as arg, refactor for single responsibility
- mkdocs, installation.md
  - condense installation information to single page
  - reference new notebook
  - remove local and production subpages due to redundancy
- environment and env_position.yml: add install current dir to avoid
  additional step in installation process
- notebooks: rewrite with Docker optional and minirec as demo data
- common_lab: raise error for invalid name
- common_position: get raw dir from settings, not hardcode
- settings.py: Should this be a class with properties?
  - add options for kachery dirs set via same dj_config mechanism
  - add raw_dir helper function

* remove note to self

* WIP notebook edits

* Revise 04_LFP nb

* Reorder/revise notebooks; #609

* Notebook formatting

* Remove old

* jupytext backup note

* Blackify py scrips. Continue config changes

* WIP: notebooks, plus improved merge_delete_downstream

* WIP: PositionSource add part table

* Refactor trodes position #613

* WIP: Fix Trodes Video

* WIP: Spellcheck. Remove debug params. Remove assigned lambda E713

* WIP: Pass tests. Remove codespell offending link

* WIP: blackify

* Selective fetch from cbroz/master

* Fetch additional file from cbroz1/master to pass CI/CD

* Add RawPos fetch method implementations. Object -> PosObject

* Refactor PosIntervalMap helpers

* Revert typo

* Bugfixes for ripple

* Blackify

* WIP: minor edits

* Add restriction to fetch1_dataframe

* Update Trodes notebook, revise others

* Set pos id default for migration. Rename Trodes params

* Fix typos

* Spelling; Jupytext sync; Blackify

* Edit gitignore for new notebook numbering

* Update changelog and notebooks

* Refactor position helpers

* Minor position_trodes fixes
edeno pushed a commit that referenced this issue Oct 12, 2023
@CBroz1
Minor fixes - docs build, refactor common (#654)
0d29760 
* Restructure for mkdocs

* Minor docstring edits for mkdocs

* Adjust installer docs

* Permit json config

* Update docstrings for mkdocs. Add images

* Minor fixes. Rename publish action. Add changelog notes

* hard wrap changes, markdownlint

* blackify

* Fix version cmd. Minor doc wording

* mkdocstrings require empty inits to identify submodules

* 🤦‍♂️ Publish docs CI/CD `main`->`master`

* Edit `get_part`; Add `merge_fetch`

* Spelling fixes

* Refactor restrict_parts. Adjust mkdocs nav.

* Docs adjust for Merge tables

* Fix `merge_get_part`

* Use hatch for docs version

* Update changelog

* Typo

* Spellcheck config

* WIP: dict/str ristrict consistency

* Normalize restriction/classmeth. Add notes on why

* Typos

* Docs publish on tag

* Edit changelog: Add links, patch version bump

* 🧪 Test gh-actions debug

* get-part multi-source flag

* Add mutual exclusivity flag from pos branch

* See details. Notebook work, config overhaul

gitignore: add exclude example config
mkdocs: new notebook names
notebooks: complete revamp for minirec data and more links to docs
init: add new load_config, isort imports
common_lab:
	- adjust to accept names in Last, First format (nwb-compliant)
	- continue to use First Last name structure in database - yes?
common_nwb: use new load_config, change `assert` to `raise`
insert_sessions: permit paths, use file name, use raw dir
storage_dirs: remove redundant funcs for base_dir
settings: implement new base_dir system
	- allows base/raw/etc to be independent
	- defaults to dj.config, then env vars, then sets default rel paths

* WIP: fix failing tests related to base_dir edits

* underscore-prefix Merge. Linter fixes

* See Details. Notebook overhaul

- dj_config: accept base dir as arg, refactor for single responsibility
- mkdocs, installation.md
  - condense installation information to single page
  - reference new notebook
  - remove local and production subpages due to redundancy
- environment and env_position.yml: add install current dir to avoid
  additional step in installation process
- notebooks: rewrite with Docker optional and minirec as demo data
- common_lab: raise error for invalid name
- common_position: get raw dir from settings, not hardcode
- settings.py: Should this be a class with properties?
  - add options for kachery dirs set via same dj_config mechanism
  - add raw_dir helper function

* remove note to self

* WIP notebook edits

* Revise 04_LFP nb

* Reorder/revise notebooks; #609

* Notebook formatting

* Remove old

* jupytext backup note

* Blackify py scrips. Continue config changes

* Refactor common

* WIP: notebooks, plus improved merge_delete_downstream

* WIP: PositionSource add part table

* Refactor trodes position #613

* WIP: Fix Trodes Video

* WIP: Spellcheck. Remove debug params. Remove assigned lambda E713

* WIP: Pass tests. Remove codespell offending link

* WIP: blackify

* Selective fetch from cbroz/master

* Fetch additional file from cbroz1/master to pass CI/CD

* Add RawPos fetch method implementations. Object -> PosObject

* Refactor PosIntervalMap helpers

* Revert typo

* Bugfixes for ripple

* Blackify

* WIP: minor edits

* Add restriction to fetch1_dataframe

* Update Trodes notebook, revise others

* Set pos id default for migration. Rename Trodes params

* Fix typos

* Spelling; Jupytext sync; Blackify

* Edit gitignore for new notebook numbering

* Update changelog and notebooks

* Refactor position helpers

* Minor position_trodes fixes

* Docs fixes

* Minor docs nav rename

* Fix typos

* Config to OOP. And #365

* Fix failing test from prev commit. Also #585

* Fixes from review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@CBroz1 CBroz1

Labels
position
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#613 - RawPosition handle multiple pynwb position objects CBroz1/spyglass
4 participants
@lfrank @edeno @CBroz1 @dpeg22