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

NF: Add keyword_only decorator to enforce keyword-only arguments #888

Merged
merged 11 commits into from
Jul 10, 2024
Merged
259 changes: 182 additions & 77 deletions fury/actor.py

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions fury/actors/odf_slicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,14 @@ def _generate_color_for_vertices(self, sf):
if self.colormap is None:
raise IOError("if global_cm=True, colormap must be defined.")
else:
all_colors = create_colormap(sf.ravel(), self.colormap) * 255
all_colors = create_colormap(sf.ravel(), name=self.colormap) * 255
elif self.colormap is not None:
if isinstance(self.colormap, str):
# Map ODFs values [min, max] to [0, 1] for each ODF
range_sf = sf.max(axis=-1) - sf.min(axis=-1)
rescaled = sf - sf.min(axis=-1, keepdims=True)
rescaled[range_sf > 0] /= range_sf[range_sf > 0][..., None]
all_colors = create_colormap(rescaled.ravel(), self.colormap) * 255
all_colors = create_colormap(rescaled.ravel(), name=self.colormap) * 255
else:
all_colors = np.tile(
np.array(self.colormap).reshape(1, 3),
Expand Down
28 changes: 19 additions & 9 deletions fury/colormap.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from scipy import linalg

from fury.data import DATA_DIR
from fury.decorators import warn_on_args_to_kwargs
from fury.lib import LookupTable

# Allow import, but disable doctests if we don't have matplotlib
Expand All @@ -14,7 +15,9 @@
cm, have_matplotlib, _ = optional_package("matplotlib.cm")


@warn_on_args_to_kwargs()
def colormap_lookup_table(
*,
scale_range=(0, 1),
hue_range=(0.8, 0),
saturation_range=(1, 1),
Expand Down Expand Up @@ -242,7 +245,8 @@ def orient2rgb(v):
return orient


def line_colors(streamlines, cmap="rgb_standard"):
@warn_on_args_to_kwargs()
def line_colors(streamlines, *, cmap="rgb_standard"):
"""Create colors for streamlines to be used in actor.line.

Parameters
Expand Down Expand Up @@ -308,7 +312,8 @@ def simple_cmap(v):
return simple_cmap


def create_colormap(v, name="plasma", auto=True):
@warn_on_args_to_kwargs()
def create_colormap(v, *, name="plasma", auto=True):
"""Create colors from a specific colormap and return it
as an array of shape (N,3) where every row gives the corresponding
r,g,b value. The colormaps we use are similar with those of matplotlib.
Expand Down Expand Up @@ -511,7 +516,8 @@ def _lab2rgb(lab):
return _xyz2rgb(tmp)


def distinguishable_colormap(bg=(0, 0, 0), exclude=None, nb_colors=None):
@warn_on_args_to_kwargs()
def distinguishable_colormap(*, bg=(0, 0, 0), exclude=None, nb_colors=None):
"""Generate colors that are maximally perceptually distinct.

This function generates a set of colors which are distinguishable
Expand Down Expand Up @@ -903,7 +909,8 @@ def get_xyz_coords(illuminant, observer):
) from err


def xyz2lab(xyz, illuminant="D65", observer="2"):
@warn_on_args_to_kwargs()
def xyz2lab(xyz, *, illuminant="D65", observer="2"):
"""XYZ to CIE-LAB color space conversion.

Parameters
Expand Down Expand Up @@ -950,7 +957,8 @@ def xyz2lab(xyz, illuminant="D65", observer="2"):
return np.concatenate([x[..., np.newaxis] for x in [L, a, b]], axis=-1)


def lab2xyz(lab, illuminant="D65", observer="2"):
@warn_on_args_to_kwargs()
def lab2xyz(lab, *, illuminant="D65", observer="2"):
"""CIE-LAB to XYZcolor space conversion.

Parameters
Expand Down Expand Up @@ -1001,7 +1009,8 @@ def lab2xyz(lab, illuminant="D65", observer="2"):
return out


def rgb2lab(rgb, illuminant="D65", observer="2"):
@warn_on_args_to_kwargs()
def rgb2lab(rgb, *, illuminant="D65", observer="2"):
"""Conversion from the sRGB color space (IEC 61966-2-1:1999)
to the CIE Lab colorspace under the given illuminant and observer.

Expand All @@ -1028,10 +1037,11 @@ def rgb2lab(rgb, illuminant="D65", observer="2"):
This implementation might have been modified.

"""
return xyz2lab(rgb2xyz(rgb), illuminant, observer)
return xyz2lab(rgb2xyz(rgb), illuminant=illuminant, observer=observer)


def lab2rgb(lab, illuminant="D65", observer="2"):
@warn_on_args_to_kwargs()
def lab2rgb(lab, *, illuminant="D65", observer="2"):
"""Lab to RGB color space conversion.

Parameters
Expand All @@ -1057,4 +1067,4 @@ def lab2rgb(lab, illuminant="D65", observer="2"):
This implementation might have been modified.

"""
return xyz2rgb(lab2xyz(lab, illuminant, observer))
return xyz2rgb(lab2xyz(lab, illuminant=illuminant, observer=observer))
10 changes: 8 additions & 2 deletions fury/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

import numpy as np

from fury.decorators import warn_on_args_to_kwargs
from fury.io import load_image


@warn_on_args_to_kwargs()
def matplotlib_figure_to_numpy(
fig, dpi=100, fname=None, flip_up_down=True, transparent=False
fig, *, dpi=100, fname=None, flip_up_down=True, transparent=False
):
"""Convert a Matplotlib figure to a 3D numpy array with RGBA channels.
Expand Down Expand Up @@ -54,7 +56,11 @@ def matplotlib_figure_to_numpy(
arr = load_image(fname)
else:
fig.savefig(
fname, dpi=dpi, transparent=transparent, bbox_inches="tight", pad_inches=0
fname,
dpi=dpi,
transparent=transparent,
bbox_inches="tight",
pad_inches=0,
)
arr = load_image(fname)

Expand Down
133 changes: 133 additions & 0 deletions fury/decorators.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
"""Decorators for FURY tests."""

from functools import wraps
from inspect import signature
import platform
import re
import sys
from warnings import warn

from packaging import version

skip_linux = is_linux = platform.system().lower() == "linux"
skip_osx = is_osx = platform.system().lower() == "darwin"
Expand Down Expand Up @@ -43,3 +48,131 @@ def doctest_skip_parser(func):
new_lines.append(code)
func.__doc__ = "\n".join(new_lines)
return func


def warn_on_args_to_kwargs(from_version="0.1.O", until_version="0.11.0"):
"""Decorator to enforce keyword-only arguments.

This decorator enforces that all arguments after the first one are
keyword-only arguments. It also checks that all keyword arguments are

skoudoro marked this conversation as resolved.
Show resolved Hide resolved
expected by the function.

Parameters:
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
-----------
from_version: str
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
The version of fury from which the function was supported.
until_version: str
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
The version of fury until which the function was supported.

Returns:
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
--------
decorator: function
Decorator function.
"""

def decorator(func):
"""Decorator
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved

Parameters:
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
-----------
func: function
Function to be decorated.

Returns:
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
--------
wrapper: function
Decorated function.

Examples:
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
---------
>>> @warn_on_args_to_kwargs()
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
... def f(a, b, *, c, d=1, e=1):
... return a + b + c + d + e
>>> f(1, 2, 3, 4, 5)
15
>>> f(1, 2, c=3, d=4, e=5)
15
>>> f(1, 2, 2, 4, e=5)
14
>>> f(1, 2, c=3, d=4)
11
>>> f(1, 2, d=3, e=5)
Traceback (most recent call last):
...
TypeError: f() missing 1 required keyword-only argument: 'c'
"""

@wraps(func)
def wrapper(*args, **kwargs):
sig = signature(func)
params = sig.parameters
#
KEYWORD_ONLY_ARGS = [
arg.name for arg in params.values() if arg.kind == arg.KEYWORD_ONLY
]
POSITIONAL_ARGS = [
arg.name
for arg in params.values()
if arg.kind in (arg.POSITIONAL_OR_KEYWORD, arg.POSITIONAL_ONLY)
]

# Keyword-only arguments that do not have default values and not in kwargs
missing_kwargs = [
arg
for arg in KEYWORD_ONLY_ARGS
if arg not in kwargs and params[arg].default == params[arg].empty
]

# Keyword-only arguments that have default values
ARG_DEFAULT = [
arg
for arg in KEYWORD_ONLY_ARGS
if arg not in kwargs and params[arg].default != params[arg].empty
]
func_params_sample = []

# Create a sample of the function parameters
for arg in params.values():
if arg.kind in (arg.POSITIONAL_OR_KEYWORD, arg.POSITIONAL_ONLY):
func_params_sample.append(f"{arg.name}_value")
elif arg.kind == arg.KEYWORD_ONLY:
func_params_sample.append(f"{arg.name}='value'")
func_params_sample = ", ".join(func_params_sample)
args_kwargs_len = len(args) + len(kwargs)
params_len = len(params)
try:
return func(*args, **kwargs)
except TypeError as e:
# if the version of fury is greater than until_version, an error should
# be displayed to indicate that this way of calling the function func
# was supported by from_version until_version but not by the current
# FURY_VERSION.
if from_version is not None and until_version is not None:
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved
from fury import __version__ as FURY_VERSION
WassCodeur marked this conversation as resolved.
Show resolved Hide resolved

if version.parse(FURY_VERSION) > version.parse(until_version):
raise TypeError(e) from e
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the other cases ? Look and review @deka27 PR.

if version.parse(FURY_VERSION) < version.parse(from_version), no warnings no errors


if ARG_DEFAULT:
missing_kwargs += ARG_DEFAULT
if missing_kwargs and params_len >= args_kwargs_len:
positional_args_len = len(POSITIONAL_ARGS)
args_k = list(args[positional_args_len:])
args = list(args[:positional_args_len])
kwargs.update(dict(zip(missing_kwargs, args_k)))
result = func(*args, **kwargs)
warn(
f"We'll no longer accept the way you call the {func.__name__} "
f"function in future versions of FURY.\n"
"Here's how to call the Function {}: {}({})".format(
func.__name__, func.__name__, func_params_sample
),
UserWarning,
stacklevel=3,
)
return result

return wrapper

return decorator
Loading
Loading