Add support for the mf4 io format

[danielhrisca]

implements #506

(Closes #506)

[danielhrisca]

This is a work in progress for which I would like to get some feed-back.

The mf4 io requires Python >= 3.6.0

[codecov]

Codecov Report

Merging #554 into mf4-io will decrease coverage by 4.27%.
The diff coverage is 78.26%.

@@            Coverage Diff             @@
##           mf4-io     #554      +/-   ##
==========================================
- Coverage   68.74%   64.46%   -4.28%     
==========================================
  Files          69       67       -2     
  Lines        6245     6098     -147     
==========================================
- Hits         4293     3931     -362     
- Misses       1952     2167     +215

[felixdivo]

Nice addition, thank you!

If this part of the PR is done, we should also focus on writing a small passage for the docs & have a look on how this can be tested. If only a writer is implemented, we could place a sample file in python-can/test/data/ and test whether all messages that we know of are in there an can be read (in the correct order).

[felixdivo]

You can also add this hot new feature to README.rst#features.

[felixdivo]

This looks like a great addition so far! :)

[danielhrisca]

@felixdivo

I've updated the listener code and now the tests pass:

test/logformats_test.py::TestCsvFileFormat::test_append_mode PASSED                                                                    [ 32%]
test/logformats_test.py::TestCsvFileFormat::test_file_like_context_manager PASSED                                                      [ 32%]
test/logformats_test.py::TestCsvFileFormat::test_file_like_explicit_stop PASSED                                                        [ 33%]
test/logformats_test.py::TestCsvFileFormat::test_path_like_context_manager PASSED                                                      [ 33%]
test/logformats_test.py::TestCsvFileFormat::test_path_like_explicit_stop PASSED                                                        [ 34%]
test/logformats_test.py::TestMF4FileFormat::test_append_mode SKIPPED                                                                   [ 35%]
test/logformats_test.py::TestMF4FileFormat::test_file_like_context_manager PASSED                                                      [ 35%]
test/logformats_test.py::TestMF4FileFormat::test_file_like_explicit_stop PASSED                                                        [ 36%]
test/logformats_test.py::TestMF4FileFormat::test_path_like_context_manager PASSED                                                      [ 36%]
test/logformats_test.py::TestMF4FileFormat::test_path_like_explicit_stop PASSED                                                        [ 37%]
test/logformats_test.py::TestSqliteDatabaseFormat::test_append_mode PASSED                                                             [ 38%]
test/logformats_test.py::TestSqliteDatabaseFormat::test_file_like_context_manager SKIPPED                                              [ 38%]
test/logformats_test.py::TestSqliteDatabaseFormat::test_file_like_explicit_stop SKIPPED                                                [ 39%]
test/logformats_test.py::TestSqliteDatabaseFormat::test_path_like_context_manager PASSED                                               [ 40%]
test/logformats_test.py::TestSqliteDatabaseFormat::test_path_like_explicit_stop PASSED                                                 [ 40%]
test/logformats_test.py::TestSqliteDatabaseFormat::test_read_all PASSED                                                                [ 41%]

Once I release asammdf 5.5.0 everything will be in place for the MF4 io support

[felixdivo]

The tests are not executed on Travis CI or AppVeyor yet. You need to:

• in .travis.yml change travis_retry pip install .[test] to travis_retry pip install .[test,mf4], and
• in .appveyor.yml change python -m pip install .[test,neovi] to python -m pip install .[test,neovi,mf4].

This should probably be added to the docs as a note, like:

.. note:: MF4 support requires Python >= 3.6 and has to be installed as an extra with for example pip install python-can[mf4].

[felixdivo]

Do we want to simply not support the combination of Pypy + MF4?

[felixdivo]

Related: travis-ci/travis-ci#9929 (comment)

[felixdivo]

Maybe this is not the right way to configure the tests. Maybe we should simply not support the combination of MF4 & PyPy (as mentioned above). This could be done by only installing travis_retry pip install .[test] and instead changing setup.py to:

import platform
is_cpython = 'CPython' == platform.python_implementation()

tests_require = [
    'pytest~=4.3',
    'pytest-timeout~=1.3',
    'pytest-cov~=2.6',
    'codecov~=2.0',
    'six',
    'hypothesis'
] + extras_require['serial']

if is_cpython:
    tests_require += extras_require['mf4']

A very simple not in the docs should be added as well:
.. note:: *MF4* is currently only supported on CPython and not on PyPy.

[altendky]

https://www.python.org/dev/peps/pep-0508/#environment-markers

'mf4; platform_python_implementation == "CPython"'

Maybe? Untested, but definitely want to get away from arbitrary code relating to setup. Also maybe != "PyPy" depending on intent.

[felixdivo]

@altendky This does not seem to work on my machine: I get python-can 3.2.0 does not provide the extra 'mf4' on CPython 3.6. Removing the spaces did not help either.

[felixdivo]

@danielhrisca Sorry for the long delay. With the above change, this should be ready to finally be merged.

[altendky]

@felixdivo, I think maybe I misread the code there. Sorry. The point is to document dependencies in a way that can be retrieved regardless of what Python is being used to run setup.py. For example, what I wrote (or something like it) can go into a wheel intact and be processed when the wheel is installed. if is_cpython: would be processed when the wheel is built and then when the wheel is used it would be fixed according to the Python used to create the wheel. This is also quite relevant to locking dependencies.

But... I was thinking at a different layer. I don't know that there's a tidy all around solution for this yet. Applying the environment markers to each dependency of the mf4 extra could make it a no-op at least and then it could be unconditionally (code-wise) included wherever. Maybe that's what I would do, I'm not quite sure.

It would be nice if the extra could be made conditionally available but my naive attempt didn't work.

https://gist.github.com/altendky/357e6a19691912593080f18ddcc39235

It would also be nice if setup.py could reference its own extras as dependencies like .[mf4] or such but last I played with that I didn't find anything that worked.

[felixdivo]

I got that idea, but it's largely unsupported for now. :( I was just trying what is implemented and what not. That shortcoming of setup.py is really annoying as well. -.-

[felixdivo]

I raised a question about this over here.

[felixdivo]

I would propose dropping OSX + Python 3.7-dev from our Travis CI list. @hardbyte Are you okay with that?

This is probably due to the python binary file. You are using 3.7-dev, which is most likely not the 3.7 release (of any version). We have not compiled the mac archives in months (if not years). At the moment, we have no plans to resume compiling them. It is best to remove these rogue archives, so as not to confuse users.

[felixdivo]

#622 is now merged. Let's see if everything except the PyPy stuff will work.

[felixdivo]

@danielhrisca Do you have time to finish this?

[danielhrisca]

@felixdivo
the pypy errors seems related to this https://travis-ci.community/t/xenial-pypy3-image-includes-broken-numpy/3523

[felixdivo]

Apparently the bug is not being cared about or fixed. ☹️ I would suggest disabling the PyPy support for now and opening an issue to change it once the above bug is fixed.

[felixdivo]

I merged this into a new branch to apply some changes, and will merge that one into develop in #697.