Skip to content

Conversation

@h-mayorquin
Copy link
Contributor

Motivation

Currently the error does not give the shape which is, I think, crucial context for the users.

The RP improves in this direction and also provides a suggestion.

@bendichter
Copy link
Contributor

@h-mayorquin you've changed "Time should be in the first dimension, and is usually the longest dimension." to "Time should be the longest dimension, which is usually the first.". The first sentence is correct. The second implies that time is always longest (which is not true) and that it can sometimes not be first (which is also not true)

@h-mayorquin
Copy link
Contributor Author

@h-mayorquin you've changed "Time should be in the first dimension, and is usually the longest dimension." to "Time should be the longest dimension, which is usually the first.". The first sentence is correct. The second implies that time is always longest (which is not true) and that it can sometimes not be first (which is also not true)

My bad, corrected it now to use the previous message but printing the shape and a suggestion.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.63%. Comparing base (3f4c834) to head (e48240a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #638      +/-   ##
==========================================
+ Coverage   73.03%   76.63%   +3.60%     
==========================================
  Files          47       47              
  Lines        1587     1588       +1     
==========================================
+ Hits         1159     1217      +58     
+ Misses        428      371      -57     
Files with missing lines Coverage Δ
src/nwbinspector/checks/_time_series.py 91.48% <100.00%> (+0.09%) ⬆️

... and 4 files with indirect coverage changes

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

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.

3 participants