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

Tidy position #870

Merged
merged 24 commits into from
Jun 26, 2024
Merged

Tidy position #870

merged 24 commits into from
Jun 26, 2024

Conversation

CBroz1
Copy link
Member

@CBroz1 CBroz1 commented Mar 12, 2024

Description

This PR primarily seeks to reduce complexity in the position pipeline to prepare for future expansion. To that end ...

  • pyproject.toml specifies the headless version of opencv for testing make_video
  • position/position_merge.py - removed unused imports as holdover from Remove classes for usused tables #1003
  • within position/v1/ ...
    • dlc_decorators.py served limited utility to enforce the source enum present in DLCModelSource
    • dlc_utils.py
      • OutputLogger context was replaced by a file_log decorator that serves the same purpose without requiring the additional indent
      • _convert_mp4 had some redundancy eliminated by extracting components into separate functions
      • utility functions from other scripts were migrated here
    • dlc_utils_makevid.py was added to collapse similar functions across DLC and Trodes implementations
    • various make functions were de-indented because of new logging decorator
  • Across a handful of places, I've added deprecation warning lines that also monitor usage of functions not directly called in the code base. If unused, these functions should be deprecated. These include
    • dlc_reader.py - Complex class that should be refactored
    • dlc_utils.py - directory lookup functions that are holdovers from element_deeplabcut
    • The log itself is stored in common_usage.py

Changes elsewhere include...

  • Adding spellchecking to the tests directory
  • Adjusting some tests to reflect new error types (eg AssertionError -> FileNotFoundError)
  • Adding some test imports where failures occurred from partial stepwise runs
  • populate_all_common - while debugging the VideoFile ingestion process for the test files, I
    • removed redundant calls to table populates and get_config.
    • further cleaned up log lines to put variables at the end for easier monitoring of the many log lines during ingestion
    • get_config now caches that a config does not exist, so spyglass doesn't attempt to look up the config and warn on fail to find many times in ingest
    • get_config also accepts an optional arg to determine which table called for the config file
  • PositionIntervalMap, related to pop_all redundancy, discussed in Populate should leave evidence of success state #849
    • Null entries are populated to avoid rerunning keys
    • convert_epoch_interval... changed to check for null entries, and delete/repopulate on populate_missing

This PR depends on two features fetched from #1002

  • dj_helper_fn.py:L264 - fetch_nwb limits the query_table restriction to only the pk to avoid a restriction with a python dict being run in SQL
  • tests/data_downloader.py - refactor of how to wait for downloads in progress

Checklist:

  • No. This PR should be accompanied by a release: (yes/no/unsure)
  • N/a (If release) I have updated the CITATION.cff
  • I have updated the CHANGELOG.md
  • I have run the position tests
  • N/a I have added/edited docs/notebooks to reflect the changes

@edeno edeno added the position label Mar 20, 2024
@CBroz1 CBroz1 linked an issue Mar 29, 2024 that may be closed by this pull request
@CBroz1 CBroz1 marked this pull request as ready for review June 25, 2024 15:30
@edeno edeno self-requested a review June 25, 2024 18:15
src/spyglass/position/v1/dlc_utils_makevid.py Outdated Show resolved Hide resolved
@edeno edeno merged commit fc41167 into LorenFrankLab:master Jun 26, 2024
7 checks passed
@CBroz1 CBroz1 deleted the dev branch June 26, 2024 16:13
@CBroz1 CBroz1 mentioned this pull request Jul 12, 2024
7 tasks
@CBroz1 CBroz1 mentioned this pull request Sep 11, 2024
7 tasks
CBroz1 added a commit to CBroz1/spyglass that referenced this pull request Oct 22, 2024
edeno pushed a commit that referenced this pull request Oct 29, 2024
* Use provided epoch

* Save video to temp dir

* Remove open-cv support

* WIP: Multithread, RAM hungry

* Limit number of workers

* Save file images in batches

* Reduce RAM cost, remove cv2 dep

* Update changelog

* Get debug arg from params

* Revert merge error #870, #975

* Adjust for final frame. Resume from existing

* Resume from fail

* except IndexError for final frame

* Delay delete temp files until complete

* Explicit error messages

* Return video object for debugging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants