Fix wrongly triggered compression check

[h-mayorquin]

Motivation

Fix #551

This also creates a definition for the size of the field/dataset and removes it from the if condition. I feel the other error got through partly because the if condition was a bit complicated and it is easy to miss logic the more difficult your assertions are.

[stephprince]

thanks for catching @h-mayorquin!

It looks like we don't have any tests for the large dataset compression warning. Would you be able to add one?

I think replicating test_check_small_dataset_compression_above_manual_threshold using the gb_lower_bound option for check_large_dataset_compression would be fine to avoid creating a large dataset in the tests.

[h-mayorquin]

Check the tests that I added.

Question: have you consider using pytest instead of unittest framework? I feel that using temporary files and directories is more friendly there:

https://docs.pytest.org/en/stable/how-to/tmp_path.html

This avoids intermittent errors on windows that result from naively trying to remove files on the test sometimes.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.77%. Comparing base (fc7491b) to head (bc9307d).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #552      +/-   ##
==========================================
+ Coverage   82.72%   86.77%   +4.04%     
==========================================
  Files          47       47              
  Lines        1511     1512       +1     
==========================================
+ Hits         1250     1312      +62     
+ Misses        261      200      -61     

Files with missing lines	Coverage Δ
src/nwbinspector/checks/_nwb_containers.py	97.72% <100.00%> (+0.05%)	⬆️

... and 5 files with indirect coverage changes

[stephprince]

Looks good to me!

I'm open to switching entirely to pytest. However, it looks like many tests currently use the unittest.TestCase class or the extended TestCase class from hdmf.testing. I think we would need to investigate a bit further to see how easy it is to migrate.

[h-mayorquin]

Yep, you will probably need to change those. From my experience with other repos it is only the custom regex test for errors and warnings that is different but I also like the pytest version more for those. Plus, is easier to skip conditions.