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

Bugfixes for metadata export #80

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

t-sasatani
Copy link
Collaborator

@t-sasatani t-sasatani commented Nov 27, 2024

  • Make metadata export keys and values consistent by writing keys after getting the first header.
  • Allow float value export in metadata (before it was all rounded to int).
  • Update test data to include computed fields and float values in tests.
  • Update metadata export test to look into the full data frame (left the df.shape test anyway).

This should be a draft, but I made it a real PR for collecting test hashes.
Done. For some reason, the hashes became consistent across OS/Python versions, which is nice, but confusing.

Notes for review

  • I made the test binary a bit larger, which is kind of non-ideal. We can make this smaller so let me know if it is too large.
  • For some reason, the video test hashes are consistent now. I don't think this PR could have broke these tests, but I would appreciate having another pair of eyes.
  • I added the test for asserting the full Pandas data frame because that's easier (ignoring data type, etc.) and probably ok for this size. However, this feels rather inconsistent with video output hash testing, so please let me know if I should fix it.

📚 Documentation preview 📚: https://miniscope-io--80.org.readthedocs.build/en/80/

@coveralls
Copy link
Collaborator

coveralls commented Nov 27, 2024

Pull Request Test Coverage Report for Build 12057509820

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 77.147%

Files with Coverage Reduction New Missed Lines %
miniscope_io/stream_daq.py 1 69.7%
Totals Coverage Status
Change from base Build 11828727123: -0.07%
Covered Lines: 1060
Relevant Lines: 1374

💛 - Coveralls

@t-sasatani t-sasatani marked this pull request as ready for review November 27, 2024 20:11
@t-sasatani
Copy link
Collaborator Author

@sneakers-the-rat, there is no rush at all for this. I just requested it before I forget.

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.

2 participants