Closed roomrys closed 1 month ago
Recent modifications enhance the handling and instantiation of Video
objects within the ParallelFeaturePipeline
and AsyncVideo
classes. Both changes shift the structure transformation process to utilize a class method from the Video
class, improving encapsulation and context awareness. These updates may enhance data validation and processing, ensuring a more robust management of video data.
File | Change Summary |
---|---|
sleap/info/feature_suggestions.py , sleap/io/asyncvideo.py |
Updated the instantiation method of Video objects to use Video.cattr().structure(...) , enhancing encapsulation and context-aware data handling. |
🐇 In the code, innovations sprout,
WithVideo
’s charm, there's no doubt!
Structure's refined, like a dance,
Encapsulation gives it a chance.
Now we hop through data with glee,
Thanks to changes, just wait and see!
✨
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Attention: Patch coverage is 50.00000%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 74.50%. Comparing base (
7ed1229
) to head (5bb8d37
). Report is 36 commits behind head on develop.
Files | Patch % | Lines |
---|---|---|
sleap/info/feature_suggestions.py | 0.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
A side question is whether we even use the sleap/io/asyncvideo.py file? I only found search results for it in tests, but those tests hung up for me when running locally....
Also, codecov is reporting that feature_suggestions.py
has the untested line, but that is actually the line that causes the error in the downstream tests (L362 in these actions)
____________________________ test_video_selection _____________________________
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
File "C:\Users\runneradmin\miniconda3\envs\sleap_ci\lib\multiprocessing\pool.py", line 125, in worker
result = (True, func(*args, **kwds))
File "C:\Users\runneradmin\miniconda3\envs\sleap_ci\lib\multiprocessing\pool.py", line 48, in mapstar
return list(map(*args))
File "D:\a\sleap\sleap\sleap\info\feature_suggestions.py", line 647, in get
video = cattr.structure(video_dict, Video)
so it must be tested - unless I am missing something.
Description
In a downstream branch that updates the version of
cattrs
(to23.x.x
from the current1.1.1
) we were seeing the following error in out CI tests:. Although we had already merged a PR to handle this (Use
Video.from_filename
when structuring videos (#1905)), it seems that there were leftovercattr.structure
calls on serializedVideo
dictionaries that did not use theVideo.cattr()
created for this purpose.This PR replaces all remaining instances of
cattr.structure
onVideo
dictionaries withVideo.cattr().structure
.Types of changes
Does this address any currently open issues?
1841
1905
Outside contributors checklist
Thank you for contributing to SLEAP!
:heart:
Summary by CodeRabbit
New Features
Bug Fixes