bannanc commented 5 years ago

Due to confusion of failed tests in PR #65 I wanted to move to testing python3.6 in its own PR.

bannanc commented 5 years ago

I will merge this assuming it passes because the only change is the python version and I don't expect that to be a problem.

bannanc commented 5 years ago

The only thing that is failing here is one SMIRKS output, this makes me think it is a subtle change in sorting for some reason, but I'm glad I ran the tests independently of other changes to know for sure this is python version issue.

bannanc commented 5 years ago

I'm trying to understand what the problem with switching to python3.6.

There is only one failure, and its in one of the functions that is looped over with @pytest.mark.parametrize. It is weird that only one of these tests is failing, since if it was an issue with networkx or another dependency then they would all be failing... Here's that failure:

=================================== FAILURES ===================================
 test_smirks_frag_graph[graph9-[#6H1X3x2r5+0a,#6H2X4x2r5+0A,#8H1X2x0!r+0A:1](-;!@[#1H0X1x0!r+0A])-,:[#6H1X3x2r5+0a,#6H2X4x2r5+0A,#6H3X4x0!r+0A:2](-,:[#1H0X1x0!r+0A,#6H2X4x2r5+0A,#7H1X3x2r5+0a])-;!@[#1H0X1x0!r+0A]-[#6H1X3r5x2a,#6H2X4r5x2A,#8!rH1X2x0A;+0:1](-;!@[#1H0X1x0!r+0A])-,:[#6!rH3X4x0A,#6H1X3r5x2a,#6H2X4r5x2A;+0:2](-,:[#1!rH0X1x0A,#6H2X4r5x2A,#7H1X3r5x2a;+0])-;!@[#1H0X1x0!r+0A]] 
graph = <chemper.graphs.cluster_graph.ClusterGraph object at 0x7fd326bb5fd0>
expected = '[#6H1X3x2r5+0a,#6H2X4x2r5+0A,#8H1X2x0!r+0A:1](-;!@[#1H0X1x0!r+0A])-,:[#6H1X3x2r5+0a,#6H2X4x2r5+0A,#6H3X4x0!r+0A:2](-,:[#1H0X1x0!r+0A,#6H2X4x2r5+0A,#7H1X3x2r5+0a])-;!@[#1H0X1x0!r+0A]'
expected_compressed = '[#6H1X3r5x2a,#6H2X4r5x2A,#8!rH1X2x0A;+0:1](-;!@[#1H0X1x0!r+0A])-,:[#6!rH3X4x0A,#6H1X3r5x2a,#6H2X4r5x2A;+0:2](-,:[#1!rH0X1x0A,#6H2X4r5x2A,#7H1X3r5x2a;+0])-;!@[#1H0X1x0!r+0A]'
    @pytest.mark.parametrize('graph,expected,expected_compressed', graph_data)
    def test_smirks_frag_graph(graph, expected, expected_compressed):
        """
        Checking the smirks pattern is the easiest way to check that the
        graphs were built correctly and that new changes have not affected the output
        """
        smirks = graph.as_smirks()
        print(smirks)
>       assert smirks == expected
E       AssertionError: assert '[#6H1X3x2r5+...1H0X1x0!r+0A]' == '[#6H1X3x2r5+0...1H0X1x0!r+0A]'
E         - [#6H1X3x2r5+0a,#6H2X4x2r5+0A,#8H1X2x0!r+0A:1](-;!@[#1H0X1x0!r+0A])-,:[#6H1X3x2r5+0a,#6H2X4x2r5+0A,#6H3X4x0!r+0A:2](-,:[#1H0X1x0!r+0A,#6H1X3x2r5+0a,#6H2X4x2r5+0A])-;!@[#1H0X1x0!r+0A]
E         ?                                                                                                                                                   --------------
E         + [#6H1X3x2r5+0a,#6H2X4x2r5+0A,#8H1X2x0!r+0A:1](-;!@[#1H0X1x0!r+0A])-,:[#6H1X3x2r5+0a,#6H2X4x2r5+0A,#6H3X4x0!r+0A:2](-,:[#1H0X1x0!r+0A,#6H2X4x2r5+0A,#7H1X3x2r5+0a])-;!@[#1H0X1x0!r+0A]
E         ?                                                                                                                                         ++++++++++++++

There are a bunch of deprecation warnings of the form:

chemper/graphs/environment.py:56
  /home/travis/build/MobleyLab/chemper/chemper/graphs/environment.py:56: DeprecationWarning: invalid escape sequence \(
    """
chemper/graphs/environment.py:90
  /home/travis/build/MobleyLab/chemper/chemper/graphs/environment.py:90: DeprecationWarning: invalid escape sequence \[
    atom, a_in, a_out = _find_embedded_brackets(smirks, '\[', '\]')
chemper/graphs/environment.py:90
  /home/travis/build/MobleyLab/chemper/chemper/graphs/environment.py:90: DeprecationWarning: invalid escape sequence \]
    atom, a_in, a_out = _find_embedded_brackets(smirks, '\[', '\]')

_find_embedded_brackets use regex functions to identify where the brackets are in the SMARTS patterns. However, that code isn't used in the section that is failing so I don't think those deprecations are causing the other error.

bannanc commented 5 years ago

I'm going to work on testing python3.6 locally to see if this error happens in both places where I can maybe track the way those SMIRKS are created and see which package is the problem.

codecov[bot] commented 5 years ago

Codecov Report

Merging #68 into master will increase coverage by 0.05%. The diff coverage is 100%.

MobleyLab / chemper

Test Python 3.6 on Travis #68

Codecov Report