Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract tests to their own directory #591

Merged
merged 4 commits into from
Apr 13, 2023
Merged

Conversation

xmatthias
Copy link
Contributor

This is mostly (actually exclusively) the work of @TkTech in #500.
It's simply extracting the test refactoring part - which will simplify review and hopefully merging.

I've also removed the duplicate installation of regular requirements - which are included in requirements_test.txt already.

TkTech and others added 4 commits April 12, 2023 13:26
Source and tests should be distinct.
Use proper pytest fixtures for shared test data.
Remove two unnecessary array comparison functions.
@mrjbq7
Copy link
Collaborator

mrjbq7 commented Apr 13, 2023

Remind me what the point of this PR is? (Not being critical, just confused)

@xmatthias
Copy link
Contributor Author

xmatthias commented Apr 13, 2023

i realize my description was a bit ... rushed - sorry for that.

call it technical maintenance / improving the code structure and tests.

Tests shouldn't be mixed in with regular code - and most/all python guides about project structuring i've seen use a dedicated tests directory, to have clear separation between functional code - and test code.
(some newer ones also user a src directory for regular code - but i don't think that's necessary).

The location the tests are currently in (mixed in along with the code, within the talib directory) means that they're included in the release (in the tarball that's uploaded to pypi).

If you install ta-lib from pypi and look at the installation directory (e.g. env/lib/python3.10/site-packages/talib/) - you'll find the following:

-rw-r--r-- 1 xmatt xmatt     799 Nov 24 19:26 abstract.py
-rw-r--r-- 1 xmatt xmatt     105 Nov 24 19:26 deprecated.py
-rw-r--r-- 1 xmatt xmatt    8879 Nov 24 19:26 __init__.py
drwxr-xr-x 2 xmatt xmatt    4096 Nov 24 19:26 __pycache__
-rw-r--r-- 1 xmatt xmatt     186 Nov 24 19:26 stream.py
-rwxr-xr-x 1 xmatt xmatt 6606568 Nov 24 19:26 _ta_lib.cpython-310-x86_64-linux-gnu.so
-rw-r--r-- 1 xmatt xmatt   12992 Nov 24 19:26 test_abstract.py
-rw-r--r-- 1 xmatt xmatt   16203 Nov 24 19:26 test_data.py
-rw-r--r-- 1 xmatt xmatt    7149 Nov 24 19:26 test_func.py
-rw-r--r-- 1 xmatt xmatt    2327 Nov 24 19:26 test_pandas.py
-rw-r--r-- 1 xmatt xmatt    3074 Nov 24 19:26 test_polars.py
-rw-r--r-- 1 xmatt xmatt    2503 Nov 24 19:26 test_stream.py

The test files are unnecessary (this is installed from a release). running pytest on this directory is not really possible - or rather, not how it's designed to work).

#500 did see little progress, and personally, i know from experience how difficult it is to review a huge PR.
i therefore think it's beneficial to improve ta-lib incrementally in small, managable/reviewable PR's (but the goal should still be to have wheels available automatically/semiautomatically).

@xmatthias xmatthias changed the title Test extract Extract tests to their own directory Apr 13, 2023
@mrjbq7 mrjbq7 merged commit b2f5f53 into TA-Lib:master Apr 13, 2023
@mrjbq7
Copy link
Collaborator

mrjbq7 commented Apr 13, 2023

okay, merged.

I do want to get the cibuildwheel PR working, but I'm unsure how to test the PyPI upload feature.

@xmatthias xmatthias deleted the test_extract branch April 13, 2023 18:40
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