search

hdmf-dev / hdmf

The Hierarchical Data Modeling Framework
http://hdmf.readthedocs.io
Other
46 stars 26 forks source link

Fix warning #1116

Closed mavaylon1 closed 4 months ago

mavaylon1 commented 4 months ago

Motivation

What was the reasoning behind this change? Please explain the changes briefly.

Fixed the warning regarding the TermSetWrapper being already wrapped. This shouldn't be raised when a configuration is not loaded.

How to test the behavior?

Show how to reproduce the new behavior (can be a bug fix or a new feature)

Checklist

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.71%. Comparing base (d390c14) to head (c695bd2). Report is 27 commits behind head on dev.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #1116 +/- ## ======================================= Coverage 88.71% 88.71% ======================================= Files 45 45 Lines 9760 9760 Branches 2771 2771 ======================================= Hits 8659 8659 Misses 779 779 Partials 322 322 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mavaylon1 commented 4 months ago

Review Notes: This is a possible fix to https://github.com/hdmf-dev/hdmf/issues/1079. However, the pydantic error is not showing up anymore. I am going to wait to see the output of manually running run_all_tests

Update: It seems to work.

mavaylon1 commented 4 months ago

The container changes look good to me. What was the reason for removing the .yaml files?

They don't really need to be there since they can be generated when running the documentation.

stephprince commented 4 months ago

Got it, would it make sense to add them to the .gitignore then if they are generated by the docs?

mavaylon1 commented 4 months ago

Got it, would it make sense to add them to the .gitignore then if they are generated by the docs?

Done. The failing hdmf-zarr test is also on schedule and seems to be unrelated to this PR.