Skip to content

Conversation

@gipert
Copy link
Member

@gipert gipert commented Jun 13, 2025

Summary

  • broaden input type hints to use generic Sequence and Mapping
  • exclude numba cache files from the wheel
  • update helper annotations for field masks

Testing

  • pre-commit run --files MANIFEST.in src/lgdo/lh5/concat.py src/lgdo/lh5/iterator.py src/lgdo/lh5/_serializers/read/utils.py src/lgdo/lh5/utils.py src/lgdo/types/table.py src/lgdo/types/waveformtable.py
  • pytest -q

https://chatgpt.com/codex/tasks/task_e_684c22b12960833099916e4acb2c4668

@codecov
Copy link

codecov bot commented Jun 13, 2025

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.28%. Comparing base (daa4603) to head (b68c48e).

Files with missing lines Patch % Lines
src/lgdo/lh5/_serializers/read/utils.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   80.25%   80.28%   +0.02%     
==========================================
  Files          47       47              
  Lines        3540     3545       +5     
==========================================
+ Hits         2841     2846       +5     
  Misses        699      699              

☔ 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.

@iguinn
Copy link
Contributor

iguinn commented Jun 16, 2025

I think that in many cases where Sequence has been chosen, Collection would be a more appropriate choice. Collection includes lists and sets, while Sequence includes lists but not sets; while our old type annotations used a lot of lists, in many cases they could easily have included sets. For this reason, when I've made these kinds of changes I've preferred Collection, although there may be situations where Sequence is more appropriate.

Also this is going to generate merge conflicts with #146

@gipert
Copy link
Member Author

gipert commented Jun 17, 2025

@iguinn I forwarded your comment to Codex. what about now?

@gipert gipert marked this pull request as draft July 6, 2025 11:55
@gipert gipert requested a review from iguinn October 1, 2025 07:38
@gipert
Copy link
Member Author

gipert commented Oct 1, 2025

@iguinn could you please review this?

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.

3 participants