Skip to content

Commit

Permalink
[Deprecation] Correct cases for memory units in core.units and fix …
Browse files Browse the repository at this point in the history
…`unit_type` detection of `FloatWithUnit.from_str` (#3838)

* fix memory unit case

* fix case in comments

* handle white space

* remove debug change

* maintain case sensitivity for memory

* simplify white space removal

* add a grace period till 2025-01-01

* use safer strip

* add test for bad string

* make msg more descriptive

* remove unnecessary type cast

* extend test coverage

---------

Co-authored-by: Janosh Riebesell <[email protected]>
Co-authored-by: Shyue Ping Ong <[email protected]>
  • Loading branch information
3 people authored Jun 17, 2024
1 parent 5b84828 commit 9719845
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 18 deletions.
28 changes: 17 additions & 11 deletions pymatgen/core/units.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import collections
import re
import warnings
from collections import defaultdict
from functools import partial
from numbers import Number
Expand Down Expand Up @@ -78,16 +79,13 @@
"intensity": {"cd": 1},
"memory": {
"byte": 1,
"Kb": 1024,
"Mb": 1024**2,
"Gb": 1024**3,
"Tb": 1024**4,
"KB": 1024,
"MB": 1024**2,
"GB": 1024**3,
"TB": 1024**4,
},
}

# Accept kb, mb, gb ... as well.
BASE_UNITS["memory"].update({k.lower(): v for k, v in BASE_UNITS["memory"].items()})

# This current list are supported derived units defined in terms of powers of
# SI base units and constants.
DERIVED_UNITS: dict[str, dict] = {
Expand Down Expand Up @@ -312,6 +310,14 @@ def __init__(
unit (str | Unit): A unit. e.g. "C".
unit_type (str): A type of unit. e.g. "charge"
"""
# Check deprecated memory unit
# TODO: remove after 2025-01-01
if unit_type == "memory" and str(unit) in {"Kb", "kb", "Mb", "mb", "Gb", "gb", "Tb", "tb"}:
warnings.warn(
f"Unit {unit!s} is deprecated, please use {str(unit).upper()} instead", DeprecationWarning, stacklevel=2
)
unit = str(unit).upper()

if unit_type is not None and str(unit) not in ALL_UNITS[unit_type]:
raise UnitError(f"{unit} is not a supported unit for {unit_type}")

Expand Down Expand Up @@ -440,16 +446,16 @@ def from_str(cls, string: str) -> Self:
"""Convert string to FloatWithUnit.
Example usage:
Memory.from_str("1. Mb").
Memory.from_str("1. MB").
"""
# Extract num and unit string.
# Extract num and unit string
string = string.strip()
for _idx, char in enumerate(string):
if char.isalpha() or char.isspace():
break
else:
raise ValueError(f"Unit is missing in string {string}")
num, unit = float(string[:_idx]), string[_idx:]
num, unit = float(string[:_idx]), string[_idx:].strip()

# Find unit type (set it to None if it cannot be detected)
for unit_type, dct in BASE_UNITS.items():
Expand Down Expand Up @@ -763,7 +769,7 @@ def _my_partial(func, *args, **kwargs):
Args:
val (float): Value
unit (Unit): e.g. Kb, Mb, Gb, Tb. Must be valid unit or UnitError
unit (Unit): e.g. KB, MB, GB, TB. Must be valid unit or UnitError
is raised.
"""

Expand Down
30 changes: 23 additions & 7 deletions tests/core/test_units.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

import warnings

import pytest
from numpy.testing import assert_array_equal
from pytest import approx
Expand Down Expand Up @@ -89,20 +91,34 @@ def test_length(self):
assert x.to("cm") == approx(4.2e-08)
assert x.to("pm") == 420
assert str(x / 2) == "2.1 ang"

y = x**3
assert y == approx(74.088)
assert str(y.unit) == "ang^3"

def test_memory(self):
mega = Memory(1, "Mb")
assert mega.to("byte") == 1024**2
assert mega == Memory(1, "mb")
mega_0 = Memory(1, "MB")
assert mega_0.to("byte") == 1024**2

mega_1 = Memory.from_str("1 MB")
assert mega_1.unit_type == "memory"

mega_2 = Memory.from_str("1MB")
assert mega_2.unit_type == "memory"

mega_3 = Memory.from_str("+1.0 MB")
assert mega_0 == mega_1 == mega_2 == mega_3

same_mega = Memory.from_str("1Mb")
assert same_mega.unit_type == "memory"
def test_deprecated_memory(self):
# TODO: remove after 2025-01-01
for unit in ("Kb", "kb", "Mb", "mb", "Gb", "gb", "Tb", "tb"):
with pytest.warns(DeprecationWarning, match=f"Unit {unit} is deprecated"):
Memory(1, unit)

other_mega = Memory.from_str("+1.0 mb")
assert mega == other_mega
with warnings.catch_warnings():
warnings.simplefilter("error")
for unit in ("KB", "MB", "GB", "TB"):
Memory(1, unit)

def test_unitized(self):
@unitized("eV")
Expand Down

0 comments on commit 9719845

Please sign in to comment.