Imported compass direction #1466
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for importing CompassDirection data from NWB files by introducing a new RawCompassDirection table. The implementation extracts spatial series from CompassDirection objects, generates valid time intervals, and creates numbered interval list entries.
Key changes:
- New
RawCompassDirectiontable that ingests CompassDirection spatial series from NWB files - Automatic generation of interval lists named "compass {n} valid times" ordered by start time
- Comprehensive test coverage validating the import functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/spyglass/common/common_behav.py | Adds RawCompassDirection table with ingestion logic for CompassDirection data |
| src/spyglass/common/init.py | Exports RawCompassDirection in public API |
| src/spyglass/common/populate_all_common.py | Includes RawCompassDirection in population workflow |
| tests/data_import/test_compass_import.py | Adds test suite for CompassDirection import functionality |
| CHANGELOG.md | Documents the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1466 +/- ##
==========================================
- Coverage 71.29% 71.29% -0.01%
==========================================
Files 114 114
Lines 13274 13273 -1
==========================================
- Hits 9464 9463 -1
Misses 3810 3810 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "name": "name", | ||
| "compass_object_id": "object_id", | ||
| "valid_times": self.generate_valid_intervals_from_timeseries, | ||
| "interval_list_name": lambda obj: f"compass {obj.object_id} valid times", # unique placeholder name |
There was a problem hiding this comment.
Is obj_id a uuid or an index? The current pattern is an index here, rather than a uuid. Makes it easier to cross-ref, but harder to read
| valid_times = self_key.pop("valid_times") # remove from self key | ||
| interval_insert = { | ||
| k: v for k, v in self_key.items() if k in IntervalList.heading.names | ||
| } | ||
| return { | ||
| IntervalList: [dict(interval_insert, valid_times=valid_times)], |
There was a problem hiding this comment.
You don't return self_key, so it may not need the pop step, removing the need to add the times back in
| valid_times = self_key.pop("valid_times") # remove from self key | |
| interval_insert = { | |
| k: v for k, v in self_key.items() if k in IntervalList.heading.names | |
| } | |
| return { | |
| IntervalList: [dict(interval_insert, valid_times=valid_times)], | |
| interval_insert = { | |
| k: v for k, v in self_key.items() if k in IntervalList.heading.names | |
| } | |
| return { | |
| IntervalList: [interval_insert], |
| start_times.append(entry["valid_times"][0][0]) | ||
| order = np.argsort(start_times) | ||
| new_names = [ | ||
| f"compass {i+1} valid times" for i in range(len(old_names)) |
There was a problem hiding this comment.
So the lambda above was a placeholder but then they get indices here? Ideally, I would structure in a way to replace a null rather than replace something that looks meaningful, like making a helper that could slot into table_key_to_obj_attr. If there are roadblocks to that pattern, we might be explicit about 'placeholder' to make it easier to check which failed, if any
| name_mapping = { | ||
| old_names[order[i]]: new_names[i] for i in range(len(old_names)) | ||
| } | ||
| for entry in interval_entries: |
There was a problem hiding this comment.
same length? for i_entry, c_entry in zip(interval_entries, compass_entries):?
Description
Fixes #1439
RawCompassDirectionCompassDirectionobject as a entryIntervalListentry following the namingcompass {x} valid timesChecklist:
CITATION.cffaltersnippet for release notes.CHANGELOG.mdwith PR number and description.