Closed cbrnr closed 1 year ago
@hofaflo could you take a look at the C compiler warnings? These seem legit, so we should initialize those variables to some sensible defaults I think.
So we cannot make sleepecg.test
a package, because this causes all tests to fail? Maybe this directory (as well as sleepecg.classifiers
) shouldn't be a package after all? Please help, I have no idea what I'm doing 😆!
@hofaflo do you know why making classifiers
and test
packages breaks the tests on cibuildwheel? Locally, all tests still pass with these changes...
Originally, this warning was generated by the conda build process in https://github.com/conda-forge/sleepecg-feedstock/pull/19:
############################
# Package would be ignored #
############################
Python recognizes 'sleepecg.test' as an importable package,
but it is not listed in the `packages` configuration of setuptools.
'sleepecg.test' has been automatically added to the distribution only
because it may contain data files, but this behavior is likely to change
in future versions of setuptools (and therefore is considered deprecated).
Please make sure that 'sleepecg.test' is included as a package by using
the `packages` configuration field or the proper discovery methods
(for example by using `find_namespace_packages(...)`/`find_namespace:`
instead of `find_packages(...)`/`find:`).
You can read more about "package discovery" and "data files" on setuptools
documentation page.
But cibuildwheel does not generate this warning, so I wonder what we should do.
OK, got it, using find_namespace:
instead of find:
in setup.cfg
solves this issue.
However, I still wonder about the error we got with pytest
when these folders are true packages.
And it would be nice if we could address the C compiler warnings here as well.
Other than that, this is ready for merge. @hofaflo please merge once you're happy.
Thanks for fixing the setuptools issue! I have no idea what's happening with pytest in cibuildwheel :/
There aren't really sensible defaults for those variables as they are only used after being set anyway (but apparently the code is too convoluted to have gcc figure that out :D). I set them to 0 and added a comment, is that okay with you?
Sure, initializing with zero is fine!
Really weird stuff, why did you choose to make these two folders not proper packages in the first place? At least the test folder is very often a package with __init__.py
from what I've seen in other projects.
I dimly remember running into this exact issue with cibuildwheel/pytest and removing the __init__.py
because of it :sweat_smile:
For classifiers
, I didn't think about making it a package because it doesn't contain any .py
-files.
It's just the tests in test_heartbeat_detection.py
that fail when we add an __init__.py
(see here for the complete log):
_____________________ test_squared_moving_integration_args _____________________
def test_squared_moving_integration_args():
"""Test squared moving window integration argument parsing."""
> from sleepecg._heartbeat_detection import _squared_moving_integration
E ModuleNotFoundError: No module named 'sleepecg._heartbeat_detection'
/project/sleepecg/test/test_heartbeat_detection.py:83: ModuleNotFoundError
All errors are related to ModuleNotFoundError: No module named 'sleepecg._heartbeat_detection'
- why could that be happening? This file is a .pyi
file, not sure if it has to do with that? It would be great if we could resolve this issue, because I have the feeling that it will hit us again in the future otherwise 😄.
Thanks @hofaflo for your support, please let's continue to discuss the ModuleNotFoundError
issue in my previous comment.
This fixes
setuptools
warnings related tosleepecg.classifiers
andsleepecg.test
, which might not get added automatically to the package in the future.In addition, I've reformatted
setup.cfg
to be as similar as possible to what it looks like whensetuptools
reformats it when building the source distribution.Fixes #143.