OBrink/RanDepict

test failures

Closed this issue · 6 comments

tulay commented

Hi @OBrink,

If you run tests with tox -e py37, there are two test failures. These errors not showing up on CI seems to be related to the line, and envlist = py38,py39,py310,lint.

FAILED Tests/test_functions.py::TestDepictionFeatureRanges::test_generate_fingerprint_schemes - AssertionError: assert 5 == 4
FAILED Tests/test_functions.py::TestDepictionFeatureRanges::test_generate_fingerprints_for_dataset - ValueError: Sum of Indigo, CDK, PIKAChU and RDKit proportions arguments has to be 1

I have created a fix on a branch on my fork that I can do a PR.
Thanks!

OBrink commented

Oh, thank you! I have implemented the usage of the feature fingerprints for PIKAChU which changed the behavior of of some of the tested functions. I am a bit irritated that the tests did not fail. It seems like my feeling of security with the passing tests was a mirage. How does the Python version in the environment affect these tests?

I went through the change and would be very happy if you could send a pull request!

  • 👍1
Kohulan commented

Maybe we could use the test environment in Tox to python 3.8 instead of 3.7.

OBrink commented

Maybe we could use the test environment in Tox to python 3.8 instead of 3.7.

The not-passing tests are linked to real problems. I did not adapt the tests after changing the behavior of some methods when implementing the depiction feature fingerprints for PIKAChU. I do not really understand why they don't fail in the Python 3.8 environment to be honest.

Kohulan commented

That is interesting. I would still recommend we should stick with python 3.8 for consistency.
Thanks a lot, @tulay for bringing this up.

OBrink commented

I have merged the changes into the main branch here. Thanks a lot @tulay!

  • 👍1
tulay commented

@Kohulan Do you want to run the tests only on py38 or all the envs listed in the tox file?
[testenv] section has no commands to run by default so actually no command is running in py38, py39 etc.

@OBrink Take a look at here. The way tox is setup in the repo only defines commands in py37 and lint.

If you really want to limit pytest runs to py37, py38 only, then you could change [testenv:py37] to [testenv:py{37,38}].

If you want to run tests on all py envs, then you could simply remove [testenv:py37] and merge its settings to [testenv]. Since [testenv:lint] overwrites command it should still work the way it is. Below is what I mean. I would prefer to run tests on all environments.

[tox]
envlist = py37, py38, py39, py310, lint
requires = tox-conda

[testenv]
setenv =
    CONDA_DLL_SEARCH_MODIFICATION_ENABLE = 1
whitelist_externals = python
conda_deps =
    pytest
    rdkit
conda_channels =
    conda-forge
commands = pytest --basetemp="{envtmpdir}" {posargs}

[testenv:lint]
....V...