Skip to content

Add support for actfast lenient mode#281

Merged
Asanto32 merged 3 commits into
mainfrom
feature/issue-280/support-actfast-lenient-mode
Jan 27, 2026
Merged

Add support for actfast lenient mode#281
Asanto32 merged 3 commits into
mainfrom
feature/issue-280/support-actfast-lenient-mode

Conversation

@Asanto32
Copy link
Copy Markdown
Collaborator

Resolves #280

Will allow sensor data to be read with actfast even if partially corrupted (data up to corrupted point is kept, see actfast lenient mode)

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 26, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.56%. Comparing base (bbbd381) to head (c999fc3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/wristpy/io/readers/readers.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
- Coverage   96.72%   96.56%   -0.17%     
==========================================
  Files          17       17              
  Lines        1100     1105       +5     
==========================================
+ Hits         1064     1067       +3     
- Misses         36       38       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Asanto32 Asanto32 marked this pull request as ready for review January 26, 2026 16:58
Comment thread src/wristpy/io/readers/readers.py Outdated
time = time.gather(unique_time_indices)

for sensor_name, sensor_values in timeseries.items():
if not isinstance(sensor_values, np.ndarray):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's happening here? Should this also throw a warning?

Copy link
Copy Markdown
Collaborator Author

@Asanto32 Asanto32 Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Flo may changed the type output from actfast because there was a mypy error that sensor_values was not necessarily an np.ndarray. This was just a type guard to just do nothing if we find something that isn't a numpy array in a sensor value so we don't try to create a Measurement on line 73 (now line 76) with those non-array values (I've never seen this happen from actfast).
I can add the warning if you think it is necessary

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nx10 ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not change any return types, but did update the type stubs to be more descriptive (as I could use newer typing features after bumping minimum python to 3.10)

You should now also got auto complete and docs on the returned dictionaries as well as basic numpy array types. (But yeah only the type hints got refined nothing changed about the implementation)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

@reindervdw-cmi reindervdw-cmi Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - then just type ignore with a comment that we believe this is a mypy issue?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to type ignore and just say, we know this is a numpy array from actfast, but I thought might as well just add this check for clarity? I'm fine with either option

Copy link
Copy Markdown
Contributor

@nx10 nx10 Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its kind of this - apparently I should annotate them with @final, but that is also broken in mypy (also doesnt work in ty, i just checked)

python/mypy#7845

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Python. Why.

Yeah either is fine to me @Asanto32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the check and ignore. Values will always be numpy arrays

@Asanto32 Asanto32 merged commit 105da6e into main Jan 27, 2026
57 of 58 checks passed
@Asanto32 Asanto32 deleted the feature/issue-280/support-actfast-lenient-mode branch January 27, 2026 16:06
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.

Task: Bump min actfast version to allow lenient mode

3 participants