8000 Allow FuncFormatter to take functions with only one field by story645 · Pull Request #17288 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Allow FuncFormatter to take functions with only one field #17288

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Allow FuncFormatter to take functions with only one field #17288

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
eed5b94
FuncFormatter can take func(x) and func(x,pos), and FuncFormatter tests
story645 May 1, 2020
103314b
flake errors
story645 May 1, 2020
14f0106
added in @anntzer's suggestion for parsing strings
story645 May 1, 2020
0ed733c
added @timhoffm's suggestion about changing wrap to lambda and implic…
story645 May 1, 2020
394d73a
@anntzer suggestions - gettattr, cbook.warn, switch in __call__
story645 May 1, 2020
a21c4a8
why are all the git checks failing?
story645 May 1, 2020
e2d5181
turned func into a property
story645 May 3, 2020
74df2a9
flake errors
story645 May 3, 2020
1232f7a
fixed tests by putting in left out closing ')'
story645 May 3, 2020
79e9063
updated to try/catch, changed the str tests, and switched polar test …
story645 May 14, 2020
bb7280e
lost a word
story645 May 14, 2020
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
26 changes: 26 additions & 0 deletions lib/matplotlib/tests/test_ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,32 @@ def test_basic(self):
assert '00002' == tmp_form(2)


class TestFuncFormatter:
tests = [("function, 1 input",
(lambda x: f"{x}!", [2], "2!")),
("function 2 inputs",
(lambda x, pos: f"{x}+{pos}!", [2, 3], "2+3!")),
("format, 1 input",
("{}!".format, [2], "2!")),
("format 2 inputs",
("{}+{}!".format, [2, 3], "2+3!"))]
ids, data = zip(*tests)
@pytest.mark.parametrize("func, args, expected", data, ids=ids)
def test_arguments(self, func, args, expected):
assert expected == mticker.FuncFormatter(func)(*args)

@pytest.mark.parametrize("func", [(lambda x, y, z: ""),
("{}+{}+{}".format)],
ids=["function", "format"])
def test_typerror(self, func):
with pytest.raises(TypeError, match=r'3 arguments'):
mticker.FuncFormatter(func)

def test_other_builtins(self):
with pytest.raises(UserWarning, match=r'max is not'):
mticker.FuncFormatter(max)


class TestStrMethodFormatter:
test_data = [
('{x:05d}', (2,), '00002'),
Expand Down
29 changes: 26 additions & 3 deletions lib/matplotlib/ticker.py
View file
Open in desktop
48F0
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,12 @@
"""

import itertools
import inspect
import logging
import locale
import math
import string
import types
from numbers import Integral

import numpy as np
Expand Down Expand Up @@ -377,19 +380,39 @@ class FuncFormatter(Formatter):
"""
Use a user-defined function for formatting.

The function should take in two inputs (a tick value ``x`` and a
position ``pos``), and return a string containing the corresponding
tick label.
The function can take in at most two inputs (a required tick value ``x``
and an optional position ``pos``), and must return a string containing
the corresponding tick label.
"""
def __init__(self, func):
self.func = func

if not isinstance(func, types.BuiltinFunctionType):
Copy link
Contributor

Choose a reason for hiding this comment

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

func is part of the public API, which means it can be changed at runtime. So you can't count on the number of arguments being the same when this is actually called. func probably needs to be changed into a property which recalculates the number of arguments in the setter.

self.nargs = len(inspect.signature(func).parameters)
elif (isinstance(getattr(func, "__self__"), str) and
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed at all? '{x}'.format(x='eggs', bar='spam') works fine, so you can just always pass both arguments.

Copy link
Member Author
@story645 story645 May 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

inspect.signature crashes on builtins, so I need to special case .format (used in lots of the polar examples, my guess is still floating in the wild too.) You're right that I probably don't need to count the arguments sent to format, but since FuncFormat only operates on two it's probably worth letting users know that. Should it be a warning instead?

(getattr(func, "__name__", "") == "format")):
#check if there's a format spec
self.nargs = len([(_, _, fs, _) for (_, _, fs, _)
in string.Formatter().parse(func.__self__)
if fs is not None])
else:
#finding argcount for other builtins is a mess
self.nargs = 2
cbook._warn_external(f"{func.__name__} is not supported "
"and may not work as expected")
if self.nargs not in [1, 2]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think best practices are to always use a set for inclusion testing (in {1, 2}). The performance benefit isn't a big deal here, but again it is a good habit to follow.

Copy link
Member

Choose a reason for hiding this comment

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

This is micro-optimization on the nanosecond level, and for the present case not even a benefit

In [2]: %timeit a in [1, 2]
63 ns ± 9.71 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [3]: %timeit a in {1, 2}
73.4 ns ± 8.28 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [4]: %timeit a in (1, 2)
61.3 ns ± 7.24 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

I think the list is slightly more readable because it's used more often, and because sets and dicts use curly braces so you have to look a bit closer to realize this is a set.

raise TypeError(f"{func.__name__} takes {self.nargs} arguments. "
"FuncFormatter functions take at most 2: "
"x (required), pos (optional).")

def __call__(self, x, pos=None):
"""
Return the value of the user defined function.

*x* and *pos* are passed through as-is.
"""
if self.nargs == 1:
return self.func(x)
return self.func(x, pos)


Expand Down
0