Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions lib/matplotlib/scale.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,25 @@ def register_scale(scale_class):
)


def unregister_scale(name):
"""
Remove a custom scale from the registry.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens if someone tries to deregister a built in scale? What should happen?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tangently I'm wondering if we should add a deregister function to the colormap registry (or is .pop good enough since the registry itself is public?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rcomer I think it should raise a "ValueError" to prevent accidental removal of built-in scales. I'll add that check to 'deregister_scale()'. Does that sound right?

@story645 That's good point! Should I address the colormap registry in this PR as well, or would it be better as a separate issue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@story645 the colormap registry has https://matplotlib.org/stable/api/cm_api.html#matplotlib.cm.ColormapRegistry.unregister

@rcomer Thanks for the reference!
Following the same pattern as 'ColormapRegistry.unregister', I'll update 'deregister_scale()" to raise a 'ValueError' if someone tries to deregister a built-in scale like 'log', 'linear', etc.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's good point! Should I address the colormap registry in this PR as well, or would it be better as a separate issue?

As @rcomer pointed out, the function already exists for colormaps. I just missed it in the docs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's good point! Should I address the colormap registry in this PR as well, or would it be better as a separate issue?

As @rcomer pointed out, the function already exists for colormaps. I just missed it in the docs.

Got it, thanks for clarifying!


Parameters
----------
name : str
The name of the scale to remove.
"""
_builtin_scales = {'linear', 'log', 'symlog', 'logit',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if this isn't defined anywhere else, I think this should be defined up top and then added to the scale registery. Use the pattern from

if name in self._builtin_cmaps:
raise ValueError(f"cannot unregister {name!r} which is a builtin "
"colormap.")

'asinh', 'function', 'functionlog'}
if name in _builtin_scales:
raise ValueError(
f"Cannot unregister built-in scale {name!r}.")
if name not in _scale_mapping:
raise ValueError(f"Scale '{name}' is not registered.")
del _scale_mapping[name]


def _get_scale_docs():
"""
Helper function for generating docstrings related to scales.
Expand Down
1 change: 1 addition & 0 deletions lib/matplotlib/scale.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,5 @@ class LogitScale(ScaleBase):
def get_scale_names() -> list[str]: ...
def scale_factory(scale: str, axis: Axis, **kwargs) -> ScaleBase: ...
def register_scale(scale_class: type[ScaleBase]) -> None: ...
def unregister_scale(name: str) -> None: ...
def _make_axis_parameter_optional(init_func: Callable[..., None]) -> Callable[..., None]: ...
45 changes: 39 additions & 6 deletions lib/matplotlib/tests/test_scale.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,7 @@ def set_default_locators_and_formatters(self, axis):
ax.set_xscale('custom')
assert isinstance(ax.xaxis.get_transform(), CustomTransform)
finally:
# cleanup - there's no public unregister_scale()
del mscale._scale_mapping["custom"]
del mscale._scale_has_axis_parameter["custom"]
mscale.unregister_scale("custom")


def test_custom_scale_with_axis():
Expand Down Expand Up @@ -368,9 +366,7 @@ def set_default_locators_and_formatters(self, axis):
ax.set_xscale('custom')
assert isinstance(ax.xaxis.get_transform(), CustomTransform)
finally:
# cleanup - there's no public unregister_scale()
del mscale._scale_mapping["custom"]
del mscale._scale_has_axis_parameter["custom"]
mscale.unregister_scale("custom")


def test_val_in_range():
Expand Down Expand Up @@ -433,3 +429,40 @@ def test_val_in_range_base_fallback():
assert s.val_in_range(np.nan) is False
assert s.val_in_range(np.inf) is False
assert s.val_in_range(-np.inf) is False


def test_unregister_scale():
"""Test that unregister_scale removes a scale correctly."""
# Register a temporary custom scale
class TempScale(mscale.ScaleBase):
name = 'temp_test_scale'

def __init__(self):
pass

def get_transform(self):
from matplotlib.transforms import IdentityTransform
return IdentityTransform()

def set_default_locators_and_formatters(
self, axis):
pass

mscale.register_scale(TempScale)
assert 'temp_test_scale' in mscale._scale_mapping

# Now unregister it
mscale.unregister_scale('temp_test_scale')
assert 'temp_test_scale' not in mscale._scale_mapping


def test_unregister_scale_invalid():
"""Test that unregister_scale raises ValueError for unknown scale."""
with pytest.raises(ValueError, match="not registered"):
mscale.unregister_scale('this_does_not_exist')


def test_unregister_scale_builtin():
"""Test that built-in scales cannot be unregistered."""
with pytest.raises(ValueError, match="Cannot unregister built-in scale"):
mscale.unregister_scale('log')
Loading