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

Add pytests to position #966

Merged
merged 42 commits into from
May 29, 2024
Merged

Add pytests to position #966

merged 42 commits into from
May 29, 2024

Conversation

CBroz1
Copy link
Member

@CBroz1 CBroz1 commented May 8, 2024

Description

This PR increases coverage of the position pipeline to ~72% when run locally.

Full coverage info
---------- coverage: platform linux, python 3.9.19-final-0 -----------
Name                                                       Stmts   Miss  Cover   Missing
----------------------------------------------------------------------------------------
src/spyglass/common/common_behav.py                          259     38    85%   47-51, 84, 198, 226, 312-316, 320-349, 352, 393-394, 429, 440, 451-457, 487, 524-525, 544, 614-616, 619-620, 646-650
src/spyglass/common/common_device.py                         202     18    91%   90, 122, 288, 391-396, 408, 540, 594-597, 658-662, 704-720
src/spyglass/common/common_dio.py                             52      3    94%   41-45, 129
src/spyglass/common/common_ephys.py                          317    181    43%   74, 110, 175-184, 209-251, 286-291, 294, 308, 380-384, 454-545, 549-558, 561-562, 613-695, 710-899, 902-903
src/spyglass/common/common_filter.py                         188     19    90%   23-24, 92-96, 99-102, 138-139, 338-341, 379, 390-398, 472
src/spyglass/common/common_interval.py                       189      4    98%   51-52, 522, 532
src/spyglass/common/common_lab.py                             91      6    93%   58-59, 232-233, 256-257
src/spyglass/common/common_nwbfile.py                        246     87    65%   210, 224, 237-238, 280-300, 344-347, 354, 361, 437-512, 539-603, 620-639, 657-658, 672, 676-681, 759, 766
src/spyglass/common/common_position.py                       269     29    89%   33-35, 190, 359-360, 367-368, 629-630, 701-767, 775-776
src/spyglass/common/common_region.py                          14      0   100%
src/spyglass/common/common_ripple.py                         122    122     0%   1-354
src/spyglass/common/common_sensors.py                         24      2    92%   36-37
src/spyglass/common/common_session.py                        105     13    88%   105, 150-154, 162, 174-177, 261-267
src/spyglass/common/common_subject.py                         25      5    80%   25-26, 34-37
src/spyglass/common/common_task.py                            82     16    80%   34-40, 120-121, 145, 178-185
src/spyglass/common/common_usage.py                          151    102    32%   75-76, 79-82, 93-103, 107, 111, 124-133, 147-152, 159, 165-170, 174, 208-210, 213-259, 263, 271-280, 284-303, 307-309, 334-381, 405-406
src/spyglass/common/errors.py                                  2      0   100%
src/spyglass/common/populate_all_common.py                    47     12    74%   40-46, 85-88, 157-159, 162-168
src/spyglass/common/prepopulate/prepopulate.py                44     31    30%   19, 24-74, 85-94
src/spyglass/common/signal_processing.py                      14     14     0%   1-53
src/spyglass/lfp/analysis/v1/lfp_band.py                     136     10    93%   108, 241, 243, 263, 277-281, 357-361, 431, 459
src/spyglass/lfp/lfp_electrode.py                             24      0   100%
src/spyglass/lfp/lfp_imported.py                              12      0   100%
src/spyglass/lfp/lfp_merge.py                                 22      0   100%
src/spyglass/lfp/v1/lfp.py                                    66      4    94%   120-124, 183-184
src/spyglass/lfp/v1/lfp_artifact.py                           55     25    55%   128-207
src/spyglass/lfp/v1/lfp_artifact_MAD_detection.py             37     29    22%   40-59, 77, 102, 124-132, 152-171
src/spyglass/lfp/v1/lfp_artifact_difference_detection.py      79     69    13%   71-223, 252-279
src/spyglass/linearization/merge.py                           14      0   100%
src/spyglass/linearization/v0/main.py                         48     24    50%   60-62, 69-70, 84-88, 122-187, 190
src/spyglass/linearization/v1/main.py                         54      9    83%   54-56, 63-64, 78-82, 187
src/spyglass/position/position_merge.py                       87     50    43%   106-107, 110-123, 139-262
src/spyglass/position/v1/dlc_decorators.py                    18      5    72%   11, 16-18, 22
src/spyglass/position/v1/dlc_reader.py                       110     24    78%   24, 38, 44-45, 51, 57-58, 61, 70, 74, 80-81, 135-137, 146, 149-162, 214, 218
src/spyglass/position/v1/dlc_utils.py                        492    265    46%   58, 61, 69, 72, 97-100, 104, 149-161, 232-235, 239-241, 246, 259, 280, 293-305, 310-316, 328-341, 356-373, 394, 405, 414, 490, 497-498, 540, 558-571, 604-611, 621-622, 651-667, 692-746, 772-782, 839, 847-849, 860-866, 889, 943, 945, 949-957, 1028-1029, 1036-1037, 1041-1299
src/spyglass/position/v1/position_dlc_centroid.py            210     11    95%   159, 173, 204-212, 219, 226, 245, 275, 704
src/spyglass/position/v1/position_dlc_cohort.py               44      1    98%   118
src/spyglass/position/v1/position_dlc_model.py               132     29    78%   35-39, 191-194, 196, 208, 281-325, 343
src/spyglass/position/v1/position_dlc_orient.py              120     32    73%   60, 112-132, 143, 220-229, 235, 249-278
src/spyglass/position/v1/position_dlc_pose_estimation.py     136      4    97%   66, 73, 120, 261
src/spyglass/position/v1/position_dlc_position.py            182     34    81%   54, 100, 198-199, 206-220, 354-356, 361, 383, 386, 408, 444-467
src/spyglass/position/v1/position_dlc_project.py             236    107    55%   128-205, 278-303, 316, 347, 361-413, 425, 476-479, 486-489, 514-555, 582, 596
src/spyglass/position/v1/position_dlc_selection.py           140      5    96%   213, 334, 378, 402, 414
src/spyglass/position/v1/position_dlc_training.py             91      5    95%   143-144, 161, 207-210
src/spyglass/position/v1/position_trodes_position.py         174      8    95%   67, 282-283, 361-362, 496, 502-503
src/spyglass/utils/database_settings.py                       94     14    85%   140, 155, 166-168, 172-176, 188, 204-207, 215
src/spyglass/utils/dj_graph.py                               458     48    90%   44, 49-50, 112, 135-137, 155, 195, 215, 241, 243, 346, 378, 444-445, 513, 556-558, 564-576, 602, 608, 642, 828, 830, 878, 884, 910, 920, 956, 965-968, 1018-1020, 1071-1073, 1087, 1110, 1123
src/spyglass/utils/dj_helper_fn.py                            91     15    84%   37, 60, 126-136, 148, 244, 253, 280-281
src/spyglass/utils/dj_merge_tables.py                        266     66    75%   37, 43-46, 74-75, 91, 153, 179, 235, 241, 323-325, 345, 351-352, 361, 412, 418, 456-462, 483-502, 525, 586, 592, 603, 650, 658, 681-682, 685, 687, 710, 721, 757-758, 768, 778-784, 788-798, 805-808, 825-836
src/spyglass/utils/dj_mixin.py                               338    115    66%   93, 107-109, 114, 121-122, 182-194, 226-237, 246-256, 292-298, 343, 414, 468-484, 489-492, 523-557, 569, 635-637, 645, 653-656, 663-665, 670-672, 687-690, 695, 699-701, 705-709, 719-721, 728-758, 807-810, 814, 818, 822, 866, 871-872, 882, 897, 901, 917-918
src/spyglass/utils/logging.py                                 17      2    88%   26-27
src/spyglass/utils/nwb_helper_fn.py                          148     27    82%   52-64, 90, 98-103, 145, 148, 157, 181, 205-206, 250, 268, 378, 491, 508, 516-534
----------------------------------------------------------------------------------------
TOTAL                                                       6576   1741    74%

Package Changes

I made an effort to avoid changes to src wherever possible. Some exceptions include...

  • common_ephys: fix typo
  • common_position.PositionVideo.make: Insert into self at the end of the make func to keep track of processed keys.
  • common_position.PositionVideo.convert_to_pixels and position.v1.position_trodes_position: in test_mode, truncate the data to avoid mismatches in length for example data
  • common_position.PositionVideo.convert_to_pixels : wrap window management call cv2.DestroyAllWindows in a try/except to handle an error hit in headless testing
  • decoding/X/dj_decoder_conversion.py: When using package scanner tools like inspect for Deprecate unused tables #976 and Merge duplicate funcs #977, I hit issues because of undefined classes in static analysis. Within the try/except ImportError, I provide default values for various classes.
  • position/v1/dlc_reader.py: In test_mode, I skip asserts that would hit on debugging. I think this class could use an overhaul to absorb other duplicative code in the pipeline, and I didn't think it worthwhile to write file-management tools for code I plan to remove later
  • position/v1/dlc_utils.py: Fix issues with my own validator.
  • Multiple: Raise errors when input conditions are violated. In general, a lot of these funcs work on dicts, which, if you're trying to write agile tests, are easy to hit as key errors or index errors
    • position.v1.position_dlc_centroid.py: Add notes where input validation is duplicated, add KeyError when params arg is missing required keys
    • position.v1.position_dlc_cohort.py: raise error when assembled table is empty, avoiding failed insert
  • position.v1.position_dlc_pose_estimation.py:
    • permit crop values as arg, rather than a required input from user, facilitating testing
    • raise error when underlying get_video_path returns None
  • position.v1.position_dlc_position.py
    • Allow test mode to mismatch analysis file version when selecting pose dataframe
    • raise error when all points below likelihood threshold to make more transparent error
  • position.v1.position_dlc_selection.py:
    • remove skip_dupe arg on insert default, which already handles this case
    • add self.insert to DLCPosVideo table to avoid reprocess duplicate keys
    • limit training to 2 iterations during testing

Actions changes

  • Add workflow_dispatch to both publish docks and tests. This will remove the need to push a dummy alpha tag just to publish docs. This also allows a contributor to run the tests on their branch before opening the pr
  • concurrency is a new feature that replaces the need for our previous 'Cancel workflow action
  • from test-conda:
    • Removed matrix - there is evidence of a previous draft using matrix to iterate over different OS, but we were not using it, and this added more clicking to get into the actual test. If we choose to revert to matrix-ed tests over py version, for example, it's worth the time saving of adding the data files as artifacts across tests
    • add mysql as a service - let gh-actions handle the mysql instance instead of doing it via docker-in-docker - I didn't collect stats, but is seems faster to let the mysql and bash instances boot concurrently
    • update Checkout, python, and miniconda steps to latest versions to get ahead of Node.js 16 deprecation
    • Install other dependencies via apt
      • mysql-client and others to send commands to server for testing 'add roles' feature of 'database_settings.py`
      • ffmpeg and others - these would be downloaded if we used the environment_dlc.yml. They allow testing a subset of features within position that are not dependent on DeepLabCut, like making output videos
  • from test-package-build
    • add 'if' qualifier in archiving so that I can run this locally via gh act
    • remove commented out portion

Supporting files

  • CODE_OF_CONDUCT: linter flagged a 'misspelling', so my markdown linter kicked in
  • docs/misc/mixin: linter flagged misspellings, added previously missing 'how to add mixin' example
  • environment- to reduce installation time in CI/CD, I ...
    • removed any item specified in pyproject.toml
    • migrated all pip-available items to pyproject.toml
    • removed the deeplabcut pin - this is a carryover from element-deeplabcut and, then, was a decision to lower maintenance burden. By pining to the old version, the install for us was slower due to resolving tensorflow dependency issues.

Tests

  • README add notes on environment variables that facilitate running new tests of dlc pipeline headlessly, and other options added to support running non-dlc tests in actions such as --no-docker and --no-dlc
  • tests/common were edited to make use of fixtures migrated to the main conftest file, shared across subpackage tests
  • DataDownloader - new class for managing data downloads. This is only used in local runs for managing the various items pulled from Box
  • tests/utils - edits made to accommodate new data population to found in merge chains, and new --no-docker means of testing role adds

Checklist:

  • No. This PR should be accompanied by a release: (yes/no/unsure)
  • n/a If release, I have updated the CITATION.cff
  • No. This PR makes edits to table definitions: (yes/no)
  • n/a If table edits, I have included an alter snippet for release notes.
  • I have updated the CHANGELOG.md with PR number and description.
  • n/a I have added/edited docs/notebooks to reflect the changes

environment_dlc.yml Outdated Show resolved Hide resolved
@CBroz1 CBroz1 marked this pull request as ready for review May 24, 2024 23:18
@edeno edeno requested a review from samuelbray32 May 24, 2024 23:21
CHANGELOG.md Outdated Show resolved Hide resolved
@edeno edeno merged commit bb4ce89 into LorenFrankLab:master May 29, 2024
7 checks passed
@CBroz1 CBroz1 deleted the ppt branch May 29, 2024 19:01
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.

None yet

3 participants