bug-fix: Numpy ValueError when cheking empty list equality #459
Conversation
Using the equality operator with an empty list will result in the following error. ValueError: operands could not be broadcast together with shapes (28,) (0,) Using len() and inverting the logic avoids this issue.
|
No idea what is going on with the test failures here, but it doesn't seem to be entirely related to the changes I made...
Most of the failures seem to be related to missing test data Maybe I missed a test in the test setup? If there are additional instructions for unit testing let me know and I'll try to figure out which of the failures are related to this pull request. |
|
I haven't looked into the errors that you're seeing, but my guess is this is either a path issue or an operating system issue. Typically I would expect to run If the approach above doesn't work, then it's possible that the tests are set up to run on a unix style operating system. If so, we would need to modify them to work on (Windows?). |
|
running pytest from the base directory does fix the file not found issues I was encountering and I'm getting much better results (or at least results that match python3.9 on debian on WSL). The failures are identical between windows and debian (WSL) now
which is encouraging. If I apply this patch and run pytest the correct way all test pass (with warnings) on both windows and debian.
So now I'm at a loss to why the CI is blowing up. |
Currently, Python 3.7 in GitHub Actions (on macos-latest) appears to
be broken ("No module named '_bz2'"). Moreover, this version has now
reached its end of life. Remove it from the test matrix.
Python 3.11 is the latest stable version; add it to the GitHub test matrix.
|
Thanks for your help!
Yeah, the GitHub workflow error reporting is not great. In this case it looks like there is something severely broken in GitHub's version of Python 3.7 on MacOS ("No module named '_bz2'"). Perhaps relatedly, CPython 3.7 is now officially at its end of life. At the same time, this numpy issue needs to be fixed in order for the other CI tests to pass. Could you please run: |
This pull request adds a changelog for `v4.2.0`. The changelog is based on the following auto-generated summary of merge commits generated by GitHub: ``` ## What's Changed * bug-fix: Numpy ValueError when cheking empty list equality by @ajadczaksunriselabs in #459 * bug-fix: Pandas set indexing error by @ajadczaksunriselabs in #460 * fix for /issues/452 by @tecamenz in #465 * Use numpydoc to render documentation by @SnoopJ in #472 * build(deps): bump readthedocs-sphinx-search from 0.1.1 to 0.3.2 in /docs by @dependabot in #477 * Update style by @bemoody in #482 * Fix NaN handling in Record.adc, and other fixes by @bemoody in #481 * Set upper bound on Numpy version (numpy = ">=1.10.1,<2.0.0"). Ref #493. by @tompollard in #494 * Update actions to use actions/checkout@v3 and actions/setup-python@v4. by @tompollard in #495 * Fix: Indent code to ensure 'j' is within for-loop in GQRS algorithm by @tompollard in #499 * Add write_dir argument to csv_to_wfdb. Fixes #67. by @tompollard in #492 * Fix warnings by @cbrnr in #502 * README improvements by @bemoody in #503 * Change in type promotion. Fixes to annotation.py by @tompollard in #506 * Use uv by @cbrnr in #504 * Change in type promotion. Fixes to _signal.py by @tompollard in #507 * Test round-trip write/read of supported binary formats by @bemoody in #509 * Corrected typo and extended allowed types for MultiSegmentRecord by @agent3gatech in #514 * Allow expanded physical signal in `calc_adc_params` by @briangow in #512 * Add capability to write signal with unique `samps_per_frame` to `wfdb.io.wrsamp` by @briangow in #510 * Fix selection of channels when converting to EDF by @SamJelfs in #519 * Change in type promotion introduced in Numpy 2.0. Fixes to edf.py. by @tompollard in #527 * Bump dependencies for NumPy 2 compatibility by @cbrnr in #511 * Bump version to v4.2.0 and update notes on creating new releases by @tompollard in #497 ## New Contributors * @ajadczaksunriselabs made their first contribution in #459 * @tecamenz made their first contribution in #465 * @SnoopJ made their first contribution in #472 * @dependabot made their first contribution in #477 * @agent3gatech made their first contribution in #514 * @SamJelfs made their first contribution in #519 **Full Changelog**: v4.1.2...v4.2.0 ```
Using the equality operator with an empty list will result in the following error.
ValueError: operands could not be broadcast together with shapes (28,) (0,)
Using len() and inverting the logic avoids this issue.