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

Remove dependency on mock and testfixtures #279

Closed
mwouts opened this issue Jul 3, 2019 · 3 comments
Closed

Remove dependency on mock and testfixtures #279

mwouts opened this issue Jul 3, 2019 · 3 comments
Milestone

Comments

@mwouts
Copy link
Owner

mwouts commented Jul 3, 2019

That was suggested by @jsd-spif: mock is available as unittest.mock in Python 3, so we don't need a dependency on mock.

Similarly, testfixtures.compare could probably be replaced with assertDictEqual from unittest.

@mwouts mwouts added this to the 1.2.0 milestone Jul 4, 2019
@mwouts
Copy link
Owner Author

mwouts commented Jul 8, 2019

Replacing mock with unitest.mock in Python 3 seems to be feasible.

Removing the dependency on testfixtures is a bit more complex. I am not sure we can use assertDictEqual directly because it is supposed to be used within unit tests. And using assert nb1 == nb2 is clearly not enough: we don't see the difference between notebooks well enough. See for instance this test:

import pytest
from nbformat.v4.nbbase import new_notebook, new_code_cell, new_markdown_cell


def notebook_metadata():
    return {
        "kernelspec": {
            "display_name": "Python 3",
            "language": "python",
            "name": "python3"
        },
        "language_info": {
            "codemirror_mode": {
                "name": "ipython",
                "version": 3
            },
            "file_extension": ".py",
            "mimetype": "text/x-python",
            "name": "python",
            "nbconvert_exporter": "python",
            "pygments_lexer": "ipython3",
            "version": "3.7.3"
        },
        "toc": {
            "base_numbering": 1,
            "nav_menu": {},
            "number_sections": True,
            "sideBar": True,
            "skip_h1_title": False,
            "title_cell": "Table of Contents",
            "title_sidebar": "Contents",
            "toc_cell": False,
            "toc_position": {},
            "toc_section_display": True,
            "toc_window_display": False
        }
    }


@pytest.fixture()
def notebook_1():
    return new_notebook(
        metadata=notebook_metadata(),
        cells=[new_markdown_cell('First markdown cell'),
               new_code_cell('1 + 1'),
               new_markdown_cell('Second markdown cell')])


@pytest.fixture()
def notebook_2():
    metadata = notebook_metadata()
    metadata['language_info']['version'] = '3.6.8'
    return new_notebook(
        metadata=metadata,
        cells=[new_markdown_cell('First markdown cell'),
               new_code_cell('1 + 1'),
               new_markdown_cell('Modified markdown cell')])


def test_notebooks_equal(notebook_1, notebook_2):
    assert notebook_1 == notebook_2

the result of which is

___________________________________________________________________________________________ test_notebooks_equal ____________________________________________________________________________________________

notebook_1 = {'nbformat_minor': 2, 'cells': [{'source': 'First markdown cell', 'cell_type':...nts_lexer': 'ipython3', 'codemirror_mode': {'version': 3, 'name': 'ipython'}}}}
notebook_2 = {'nbformat_minor': 2, 'cells': [{'source': 'First markdown cell', 'cell_type':...nts_lexer': 'ipython3', 'codemirror_mode': {'version': 3, 'name': 'ipython'}}}}

    def test_notebooks_equal(notebook_1, notebook_2):
>       assert notebook_1 == notebook_2
E       AssertionError: assert {'nbformat_mi... 'ipython'}}}} == {'nbformat_min... 'ipython'}}}}
E         Omitting 2 identical items, use -vv to show
E         Differing items:
E         {'cells': [{'source': 'First markdown cell', 'cell_type': 'markdown', 'metadata': {}}, {'source': '1 + 1', 'cell_type'... 'execution_count': None, 'outputs': []}, {'source': 'Second markdown cell', 'cell_type': 'markdown', 'metadata': {}}]} != {'cells': [{'source': 'First markdown cell', 'cell_type': 'markdown', 'metadata': {}}, {'source': '1 + 1', 'cell_type'...execution_count': None, 'outputs': []}, {'source': 'Modified markdown cell', 'cell_type': 'markdown', 'metadata': {}}]}
E         {'metadata': {'kernelspec': {'di...
E
E         ...Full output truncated (2 lines hidden), use '-vv' to show

test_compare_notebooks.py:61: AssertionError

which does not show clearly the difference between the two notebooks...

@ghost
Copy link

ghost commented Jul 8, 2019

Hello @mwouts

You are right. It is clearly feasible to rely on unittest.mock to avoid depending on mock when running jupytext under python3, but we should not remove the dependency to testfixtures due to the usage you have of the compare method.

Anyway, thanks for the feedback, I just got time last week end to write changes for the mock package. I'll open the PR soon.

mwouts added a commit that referenced this issue Jul 8, 2019
mwouts added a commit that referenced this issue Jul 8, 2019
mwouts added a commit that referenced this issue Jul 9, 2019
@mwouts
Copy link
Owner Author

mwouts commented Jul 9, 2019

Finally I was able to implement a replacement for testfixtures.compare using difflib, see this commit.

@jsd-spif , are you ok if I merge the corresponding PR now, or do you prefer to contribute your PR first?

mwouts added a commit that referenced this issue Jul 9, 2019
mwouts added a commit that referenced this issue Jul 9, 2019
mwouts added a commit that referenced this issue Jul 9, 2019
@mwouts mwouts closed this as completed Jul 9, 2019
mwouts added a commit to regro-cf-autotick-bot/jupytext-feedstock that referenced this issue Jul 18, 2019
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

No branches or pull requests

1 participant