[Deprecation] Correct cases for memory units in core.units and fix unit_type detection of FloatWithUnit.from_str

[DanielYang59]

Summary

• Correct cases for memory units in core.units to fix Incorrect letter cases for memory in core.units #3833
• Fix FloatWithUnit.from_str to correctly detect unit_type from string with space between the value and the unit

Correct cases for memory units in core.units

I assume this is another breaking fix, so I marked the incorrect units as deprecated until 2025-01-01. Any suggestion would be appreciated.

Currently the case of byte is incorrect in core.units, where B refers to "byte" and b refers to "bit".
Also note 1 byte = 8 bit so we cannot use them interchangeably.

pymatgen/pymatgen/core/units.py

Lines 77 to 83 in bb68c78

	"memory": {
	"byte": 1,
	"Kb": 1024,
	"Mb": 1024**2,
	"Gb": 1024**3,
	"Tb": 1024**4,
	},

Case sensitivity for memory units

I assume there is no good reason to remove case sensitivity for memory alone (case sensitivity of other units is maintained), especially when the case of memory has its meaning:

pymatgen/pymatgen/core/units.py

Lines 90 to 91 in 14b7357

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

Fix FloatWithUnit.from_str to correctly detect unit_type from string with space between value and unit

Previously for a string with space (which adheres to the SI Unit rules) like Memory.from_str("1 MB"), the string would be split into 1 and MB, and the space in MB prevents the unit_type being detected properly and ended up being None.

from pymatgen.core.units import Memory

mem_0 = Memory.from_str("1MB")

print(mem_0.unit_type)  # >>> "memory"

mem_1 = Memory.from_str("1 MB")

print(mem_1.unit_type)  # >>> None

[DanielYang59]

@shyuep Can I have your advice on this please? This should be very similar to the change to Element, and I scheduled the same grace period until 2025-01-01

[DanielYang59]

@shyuep Can you please review this? It should be very similar to #3817.

The rationale for this change is that something like KB (kilobyte) and kb(kilobit) refer to different units for memory (or storage in general), and as such it should not be case insensitive.

[shyuep]

This is fine. I am enabling auto-merge. There was a failing test that needs to be fixed.

[DanielYang59]

Thanks. I notice that too e572d34#r143073539, perhaps we also want to rename the id argument to avoid shadowing the builtin in e572d34#r143131235?

[shyuep]

Thanks. I renamed id to id_

[DanielYang59]

Thanks!