8000 Update matplotlibrc.template by ImportanceOfBeingErnest · Pull Request #10531 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Update matplotlibrc.template #10531

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Feb 26, 2018
Merged

Update matplotlibrc.template #10531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
675e6b5
Update matplotlibrc.template
ImportanceOfBeingErnest Feb 19, 2018
5d8284e
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
facd97a
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
dc17fe2
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
b7b3f32
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
e93c66a
Test to see if test is correctly testing. (WIP)
ImportanceOfBeingErnest Feb 19, 2018
a67b9d0
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
9964e8e
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
5cd5b29
Update test_rcparams.py
ImportanceOfBeingErnest Feb 19, 2018
7d08d3d
Update matplotlibrc.template
ImportanceOfBeingErnest Feb 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions lib/matplotlib/tests/test_rcparams.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,3 +422,64 @@ def test_rcparams_reset_after_fail():
pass

assert mpl.rcParams['text.usetex'] is False


def test_if_rctemplate_is_up_to_date():
# This tests if the matplotlibrc.template file
# contains all valid rcParams.
dep1 = mpl._all_deprecated
dep2 = mpl._deprecated_set
deprecated = list(dep1.union(dep2))
#print(deprecated)
path_to_rc = mpl.matplotlib_fname()
with open(path_to_rc, "r") as f:
rclines = f.readlines()
missing = {}
for k,v in mpl.defaultParams.items():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For better readability, I'd slightly prefer for key, val or for name, val because the for loop is quite long.

if k[0] == "_":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe join:

if k.startswith(' ') or k in deprecated or 'verbose' in k:
    continue

continue
if k in deprecated:
continue
if "verbose" in k:
continue
found = False
Copy link
Member
@timhoffm timhoffm Feb 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and following lines) can be written more compact:

if not any(k in line for line in lines):
    missing[k] = v

Note, that k in line is a bit imprecise because it would also match values or comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I think any better solution here would eventually lead to writing one single test and match the parsed file agains the defaultParams. Depends on if people think this should be done or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a big fan of "incrementally better", so I don't want to enforce a full parsing test. Your partial test is already a big improvement. Maybe you could add a short comment to clarify that this test is not expected to be 100%. And of course, use the compact form 😃.

for line in rclines:
if k in line:
found = True
if not found:
missing.update({k:v})
if missing:
raise ValueError("The following params are missing " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert not missing would be more inline with pytest conventions

assert missing == {} would also work.

Both should provide a reasonably informative error message.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but assert not missing would show exactly as assert not missing in the Error, without any details, right? Here I would want to show all missing parameters in the error. That should make it relatively easy to just copy and paste them to the file.

Copy link
Member
@phobson phobson Feb 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pytest is pretty good about reporting. Consider the following:

$ cat test_simple.py
import pytest

@pytest.fixture
def missing():
    return {'a': 1, 'b': 2}

def test_one(missing):
    assert not miss
A600
ing

def test_two(missing):
    assert missing == {}

So then running it:

============================== test session starts ===============================
platform linux -- Python 3.6.4, pytest-3.4.0, py-1.5.2, pluggy-0.6.0
Matplotlib: 2.1.2
Freetype: 2.8.1
rootdir: /mnt/c/Users/phobson/sources/scratch/simple_test, inifile:
plugins: pep8-1.0.6, mpl-0.9, cov-2.5.1
collected 2 items

test_simple.py FF                                                          [100%]

==================================== FAILURES ====================================
____________________________________ test_one ____________________________________

missing = {'a': 1, 'b': 2}

    def test_one(missing):
>       assert not missing
E       AssertionError: assert not {'a': 1, 'b': 2}

test_simple.py:9: AssertionError
____________________________________ test_two ____________________________________

missing = {'a': 1, 'b': 2}

    def test_two(missing):
>       assert missing == {}
E       AssertionError: assert {'a': 1, 'b': 2} == {}
E         Left contains more items:
E         {'a': 1, 'b': 2}
E         Use -v to get the full diff

test_simple.py:13: AssertionError
============================ 2 failed in 0.28 seconds ============================

It's pretty clear what's in the dictionary that should be empty

"in the matplotlibrc.template file: {}"
.format(missing.items()))


def test_if_rctemplate_would_be_valid(tmpdir):
# This tests if the matplotlibrc.template file would result in a valid
# rc file if all lines are uncommented.
path_to_rc = mpl.matplotlib_fname()
with open(path_to_rc, "r") as f:
rclines = f.readlines()
newlines = []
for line in rclines:
if line[0] == "#":
newline = line[1:]
else:
newline = line
if "$TEMPLATE_BACKEND" in newline:
newline = "backend : Agg"
if "datapath" in newline:
newline = ""
newlines.append(newline)
d = tmpdir.mkdir('test1')
fname = str(d.join('testrcvalid.temp'))
with open(fname, "w") as f:
f.writelines(newlines)
with pytest.warns(None) as record:
dic = mpl.rc_params_from_file(fname,
fail_on_error=True,
use_default_template=False)
assert len(record) == 0
#d1 = set(dic.keys())
#d2 = set(matplotlib.defaultParams.keys())
#print(d2-d1)
Loading
0