Repository: pydicom/pydicom

Issue:
DSfloat, DSdecimal and IS fail consistency checks
```python
from pydicom import Dataset

ds = Dataset()
ds.PatientWeight = "1.5"  # VR DS
assert ds.PatientWeight == "1.5"  # AssertionError
ds.BeamNumber = "1"  # VR IS
assert ds.BeamNumber == "1"  # AssertionError
```
Because `DSfloat`, `DSdecimal` and `IS` use `float`, `Decimal` and `int` internally the comparison to the strings fail. Suggest overriding `__eq__`, etc, methods to check `str(value) == other` when other is `str`.

```python
from pydicom import Dataset

ds = Dataset()
ds.BeamNumber = ""  # VR IS
assert ds.BeamNumber == ""  # AssertionError
ds.BeamNumber = None
assert ds.BeamNumber is None  # OK
```
When `""` is used with `IS` it gets changed to `None`, which breaks consistency with all other VR types (except SQ).

```python
>>> from pydicom.valuerep import DSfloat, DSdecimal, IS
>>> DSfloat(1)
"1.0"
>>> DSdecimal(1)
"1"
>>> IS(1)
"1"
>>> "1"
'1'
>>> repr("1")
"'1'"
>>> repr(IS("1"))
'"1"'
```
The repr (?) returns should use `'` rather than `"` to be consistent with `str`.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
> Merging [#1397](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (6f26e37) into [master](https://codecov.io/gh/pydicom/pydicom/commit/d01727ae729cdc05ee7817b56ec3bff13908464c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (d01727a) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pydicom/pydicom/pull/1397/graphs/tree.svg?width=650&height=150&src=pr&token=YciLV15Lwr&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)

```diff
@@           Coverage Diff           @@
##           master    #1397   +/-   ##
=======================================
  Coverage   95.53%   95.53%           
=======================================
  Files          65       65           
  Lines       10183    10183           
=======================================
  Hits         9728     9728           
  Misses        455      455           
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | Coverage Δ | |
|---|---|---|
| [pydicom/valuerep.py](https://codecov.io/gh/pydicom/pydicom/pull/1397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS92YWx1ZXJlcC5weQ==) | `98.74% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Last update [d01727a...6f26e37](https://codecov.io/gh/pydicom/pydicom/pull/1397?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).

scaramallion: Ready for review
darcymason: > Ready for review

I should be able to review this today

Patch:
diff --git a/.github/workflows/pr-typing.yml b/.github/workflows/pr-typing.yml
new file mode 100644
index 000000000..1331414aa
--- /dev/null
+++ b/.github/workflows/pr-typing.yml
@@ -0,0 +1,32 @@
+name: pull-request-typing
+
+on:
+  pull_request:
+    branches: [ master ]
+
+jobs:
+
+  check-typing:
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: [3.9]
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install mypy
+    - name: Enable typing
+      run: |
+        touch pydicom/py.typed
+    - name: Run typing check with mypy
+      run: |
+        mypy
diff --git a/mypy.ini b/mypy.ini
new file mode 100644
index 000000000..4060b0cf5
--- /dev/null
+++ b/mypy.ini
@@ -0,0 +1,6 @@
+[mypy]
+python_version = 3.9
+exclude = pydicom/(tests|benchmarks)
+files = pydicom/
+show_error_codes = True
+warn_redundant_casts = True
diff --git a/pydicom/valuerep.py b/pydicom/valuerep.py
index 9f2c8e577..2ef7c5b2f 100644
--- a/pydicom/valuerep.py
+++ b/pydicom/valuerep.py
@@ -16,15 +16,6 @@ from typing import (
 from pydicom import config
 from pydicom.multival import MultiValue
 
-# Types
-_T = TypeVar('_T')
-_DA = TypeVar("_DA", bound="DA")
-_DT = TypeVar("_DT", bound="DT")
-_TM = TypeVar("_TM", bound="TM")
-_IS = TypeVar("_IS", bound="IS")
-_DSfloat = TypeVar("_DSfloat", bound="DSfloat")
-_DSdecimal = TypeVar("_DSdecimal", bound="DSdecimal")
-_PersonName = TypeVar("_PersonName", bound="PersonName")
 
 # can't import from charset or get circular import
 default_encoding = "iso8859"
@@ -86,8 +77,8 @@ class DA(_DateTimeBase, datetime.date):
     Note that the :class:`datetime.date` base class is immutable.
     """
     def __new__(  # type: ignore[misc]
-        cls: Type[_DA], *args, **kwargs
-    ) -> Optional[_DA]:
+        cls: Type["DA"], *args, **kwargs
+    ) -> Optional["DA"]:
         """Create an instance of DA object.
 
         Raise an exception if the string cannot be parsed or the argument
@@ -176,8 +167,8 @@ class DT(_DateTimeBase, datetime.datetime):
         )
 
     def __new__(  # type: ignore[misc]
-        cls: Type[_DT], *args, **kwargs
-    ) -> Optional[_DT]:
+        cls: Type["DT"], *args, **kwargs
+    ) -> Optional["DT"]:
         """Create an instance of DT object.
 
         Raise an exception if the string cannot be parsed or the argument
@@ -289,8 +280,8 @@ class TM(_DateTimeBase, datetime.time):
     )
 
     def __new__(  # type: ignore[misc]
-        cls: Type[_TM], *args, **kwargs
-    ) -> Optional[_TM]:
+        cls: Type["TM"], *args, **kwargs
+    ) -> Optional["TM"]:
         """Create an instance of TM object from a string.
 
         Raise an exception if the string cannot be parsed or the argument
@@ -492,11 +483,17 @@ class DSfloat(float):
     """
     auto_format: bool
 
-    def __new__(
-        cls: Type[_DSfloat],
+    def __new__(  # type: ignore[misc]
+        cls: Type["DSfloat"],
         val: Union[str, int, float, Decimal],
         auto_format: bool = False
-    ) -> _DSfloat:
+    ) -> Optional[Union[str, "DSfloat"]]:
+        if val is None:
+            return val
+
+        if isinstance(val, str) and val.strip() == '':
+            return val
+
         return super().__new__(cls, val)
 
     def __init__(
@@ -511,7 +508,7 @@ class DSfloat(float):
         has_attribute = hasattr(val, 'original_string')
         pre_checked = False
         if isinstance(val, str):
-            self.original_string = val
+            self.original_string = val.strip()
         elif isinstance(val, (DSfloat, DSdecimal)):
             if val.auto_format:
                 auto_format = True  # override input parameter
@@ -548,6 +545,16 @@ class DSfloat(float):
                     'of DS'
                 )
 
+    def __eq__(self, other: Any) -> bool:
+        """Override to allow string equality comparisons."""
+        if isinstance(other, str):
+            return str(self) == other
+
+        return super().__eq__(other)
+
+    def __ne__(self, other: Any) -> bool:
+        return not self == other
+
     def __str__(self) -> str:
         if hasattr(self, 'original_string') and not self.auto_format:
             return self.original_string
@@ -557,8 +564,9 @@ class DSfloat(float):
 
     def __repr__(self) -> str:
         if self.auto_format and hasattr(self, 'original_string'):
-            return f'"{self.original_string}"'
-        return f'"{super().__repr__()}"'
+            return f"'{self.original_string}'"
+
+        return f"'{super().__repr__()}'"
 
 
 class DSdecimal(Decimal):
@@ -582,10 +590,10 @@ class DSdecimal(Decimal):
     auto_format: bool
 
     def __new__(  # type: ignore[misc]
-        cls: Type[_DSdecimal],
+        cls: Type["DSdecimal"],
         val: Union[str, int, float, Decimal],
         auto_format: bool = False
-    ) -> Optional[_DSdecimal]:
+    ) -> Optional[Union[str, "DSdecimal"]]:
         """Create an instance of DS object, or return a blank string if one is
         passed in, e.g. from a type 2 DICOM blank value.
 
@@ -594,6 +602,12 @@ class DSdecimal(Decimal):
         val : str or numeric
             A string or a number type which can be converted to a decimal.
         """
+        if val is None:
+            return val
+
+        if isinstance(val, str) and val.strip() == '':
+            return val
+
         if isinstance(val, float) and not config.allow_DS_float:
             raise TypeError(
                 "'DS' cannot be instantiated with a float value unless "
@@ -602,14 +616,7 @@ class DSdecimal(Decimal):
                 "or use 'Decimal.quantize()' and pass a 'Decimal' instance."
             )
 
-        if isinstance(val, str):
-            val = val.strip()
-            if val == '':
-                return None
-
-        val = super().__new__(cls, val)
-
-        return val
+        return super().__new__(cls, val)
 
     def __init__(
         self,
@@ -622,15 +629,15 @@ class DSdecimal(Decimal):
         """
         # ... also if user changes a data element value, then will get
         # a different Decimal, as Decimal is immutable.
-        has_str = hasattr(val, 'original_string')
         pre_checked = False
         if isinstance(val, str):
-            self.original_string = val
+            self.original_string = val.strip()
         elif isinstance(val, (DSfloat, DSdecimal)):
             if val.auto_format:
                 auto_format = True  # override input parameter
                 pre_checked = True
-            if has_str:
+
+            if hasattr(val, 'original_string'):
                 self.original_string = val.original_string
 
         self.auto_format = auto_format
@@ -646,7 +653,7 @@ class DSdecimal(Decimal):
                 self.original_string = format_number_as_ds(self)
 
         if config.enforce_valid_values:
-            if len(repr(self).strip('"')) > 16:
+            if len(repr(self).strip("'")) > 16:
                 raise OverflowError(
                     "Values for elements with a VR of 'DS' values must be "
                     "<= 16 characters long. Use a smaller string, set "
@@ -655,13 +662,23 @@ class DSdecimal(Decimal):
                     "with a 'Decimal' instance, or explicitly construct a DS "
                     "instance with 'auto_format' set to True"
                 )
-            if not is_valid_ds(repr(self).strip('"')):
+            if not is_valid_ds(repr(self).strip("'")):
                 # This will catch nan and inf
                 raise ValueError(
                     f'Value "{str(self)}" is not valid for elements with a VR '
                     'of DS'
                 )
 
+    def __eq__(self, other: Any) -> bool:
+        """Override to allow string equality comparisons."""
+        if isinstance(other, str):
+            return str(self) == other
+
+        return super().__eq__(other)
+
+    def __ne__(self, other: Any) -> bool:
+        return not self == other
+
     def __str__(self) -> str:
         has_str = hasattr(self, 'original_string')
         if has_str and len(self.original_string) <= 16:
@@ -671,8 +688,8 @@ class DSdecimal(Decimal):
 
     def __repr__(self) -> str:
         if self.auto_format and hasattr(self, 'original_string'):
-            return f'"{self.original_string}"'
-        return f'"{str(self)}"'
+            return f"'{self.original_string}'"
+        return f"'{str(self)}'"
 
 
 # CHOOSE TYPE OF DS
@@ -696,12 +713,10 @@ def DS(
     called directly if a string has already been processed.
     """
     if val is None:
-        return None
+        return val
 
-    if isinstance(val, str):
-        val = val.strip()
-        if val == '':
-            return ''
+    if isinstance(val, str) and val.strip() == '':
+        return val
 
     if config.use_DS_decimal:
         return DSdecimal(val, auto_format=auto_format)
@@ -717,14 +732,14 @@ class IS(int):
     """
 
     def __new__(  # type: ignore[misc]
-        cls: Type[_IS], val: Union[None, str, int, float, Decimal]
-    ) -> Optional[_IS]:
+        cls: Type["IS"], val: Union[None, str, int, float, Decimal]
+    ) -> Optional[Union[str, "IS"]]:
         """Create instance if new integer string"""
         if val is None:
             return val
 
         if isinstance(val, str) and val.strip() == '':
-            return None
+            return val
 
         try:
             newval = super().__new__(cls, val)
@@ -751,10 +766,20 @@ class IS(int):
     def __init__(self, val: Union[str, int, float, Decimal]) -> None:
         # If a string passed, then store it
         if isinstance(val, str):
-            self.original_string = val
+            self.original_string = val.strip()
         elif isinstance(val, IS) and hasattr(val, 'original_string'):
             self.original_string = val.original_string
 
+    def __eq__(self, other: Any) -> bool:
+        """Override to allow string equality comparisons."""
+        if isinstance(other, str):
+            return str(self) == other
+
+        return super().__eq__(other)
+
+    def __ne__(self, other: Any) -> bool:
+        return not self == other
+
     def __str__(self) -> str:
         if hasattr(self, 'original_string'):
             return self.original_string
@@ -763,7 +788,10 @@ class IS(int):
         return repr(self)[1:-1]
 
     def __repr__(self) -> str:
-        return f'"{super().__repr__()}"'
+        return f"'{super().__repr__()}'"
+
+
+_T = TypeVar('_T')
 
 
 def _as_str(s: str):
@@ -771,8 +799,7 @@ def _as_str(s: str):
 
 
 def MultiString(
-    val: str,
-    valtype: Optional[Callable[[str], _T]] = None
+    val: str, valtype: Optional[Callable[[str], _T]] = None
 ) -> Union[_T, MutableSequence[_T]]:
     """Split a string by delimiters if there are any
 
@@ -880,8 +907,8 @@ def _encode_personname(components, encodings):
 class PersonName:
     """Representation of the value for an element with VR **PN**."""
     def __new__(  # type: ignore[misc]
-        cls: Type[_PersonName], *args, **kwargs
-    ) -> Optional[_PersonName]:
+        cls: Type["PersonName"], *args, **kwargs
+    ) -> Optional["PersonName"]:
         if len(args) and args[0] is None:
             return None
 


Test patch:
diff --git a/pydicom/tests/test_util.py b/pydicom/tests/test_util.py
index 263899b45..a8eacc2fb 100644
--- a/pydicom/tests/test_util.py
+++ b/pydicom/tests/test_util.py
@@ -85,7 +85,7 @@ class TestCodify:
         # DataElement(0x00080301, 'US', 1200)]
         out_str = ["ds.PatientName = # XXX Array of 7 bytes excluded",
                    "ds.CodingSchemeUID = '1.1'",
-                   'ds.SeriesNumber = "3"']
+                   "ds.SeriesNumber = '3'"]
         # Fails
         # "ds.PrivateGroupReference = 1200"]
         for elem, out in zip(input_elem, out_str):
diff --git a/pydicom/tests/test_valuerep.py b/pydicom/tests/test_valuerep.py
index 1fd60cb6c..7948234f8 100644
--- a/pydicom/tests/test_valuerep.py
+++ b/pydicom/tests/test_valuerep.py
@@ -17,7 +17,8 @@ import pydicom
 from pydicom import config
 from pydicom import valuerep
 from pydicom.data import get_testdata_file
-from pydicom.valuerep import DS, IS
+from pydicom.dataset import Dataset
+from pydicom.valuerep import DS, IS, DSfloat, DSdecimal
 import pytest
 
 from pydicom.valuerep import PersonName
@@ -484,14 +485,15 @@ class TestDS:
     """Unit tests for DS values"""
     def test_empty_value(self):
         assert DS(None) is None
-        assert "" == DS("")
+        assert DS("") == ""
+        assert DS("   ") == "   "
 
     def test_float_values(self):
         val = DS(0.9)
-        assert isinstance(val, pydicom.valuerep.DSfloat)
+        assert isinstance(val, DSfloat)
         assert 0.9 == val
         val = DS("0.9")
-        assert isinstance(val, pydicom.valuerep.DSfloat)
+        assert isinstance(val, DSfloat)
         assert 0.9 == val
 
 
@@ -499,33 +501,51 @@ class TestDSfloat:
     """Unit tests for pickling DSfloat"""
     def test_pickling(self):
         # Check that a pickled DSFloat is read back properly
-        x = pydicom.valuerep.DSfloat(9.0)
+        x = DSfloat(9.0)
         x.original_string = "hello"
         data1_string = pickle.dumps(x)
         x2 = pickle.loads(data1_string)
         assert x.real == x2.real
         assert x.original_string == x2.original_string
 
+    def test_new_empty(self):
+        """Test passing an empty value."""
+        assert isinstance(DSfloat(''), str)
+        assert DSfloat('') == ''
+        assert DSfloat(None) is None
+
+    def test_str_value(self):
+        """Test creating using str"""
+        assert DSfloat('1.20') == 1.2
+        assert DSfloat('1.20') == 1.20
+        assert DSfloat('1.20 ') == 1.2
+        assert DSfloat('1.20 ') == 1.20
+        assert DSfloat('1.20') != '1.2'
+        assert DSfloat('1.20') == '1.20'
+        assert DSfloat('1.20 ') == '1.20'
+
     def test_str(self):
         """Test DSfloat.__str__()."""
-        val = pydicom.valuerep.DSfloat(1.1)
-        assert "1.1" == str(val)
+        val = DSfloat(1.1)
+        assert str(val) == '1.1'
 
-        val = pydicom.valuerep.DSfloat("1.1")
-        assert "1.1" == str(val)
+        val = DSfloat("1.1")
+        assert str(val) == '1.1'
 
     def test_repr(self):
         """Test DSfloat.__repr__()."""
-        val = pydicom.valuerep.DSfloat(1.1)
-        assert '"1.1"' == repr(val)
+        val = DSfloat(1.1)
+        assert repr(val) == "'1.1'"
 
-        val = pydicom.valuerep.DSfloat("1.1")
-        assert '"1.1"' == repr(val)
+        val = DSfloat("1.1")
+        assert repr(val) == "'1.1'"
+        assert repr(val) == repr("1.1")
+        assert repr(val) == repr('1.1')
 
     def test_DSfloat(self):
         """Test creating a value using DSfloat."""
-        x = pydicom.valuerep.DSfloat('1.2345')
-        y = pydicom.valuerep.DSfloat(x)
+        x = DSfloat('1.2345')
+        y = DSfloat(x)
         assert x == y
         assert y == x
         assert 1.2345 == y
@@ -533,54 +553,54 @@ class TestDSfloat:
 
     def test_DSdecimal(self):
         """Test creating a value using DSdecimal."""
-        x = pydicom.valuerep.DSdecimal('1.2345')
-        y = pydicom.valuerep.DSfloat(x)
+        x = DSdecimal('1.2345')
+        y = DSfloat(x)
         assert 1.2345 == y
         assert "1.2345" == y.original_string
 
     def test_auto_format(self, enforce_valid_both_fixture):
         """Test truncating floats"""
-        x = pydicom.valuerep.DSfloat(math.pi, auto_format=True)
+        x = DSfloat(math.pi, auto_format=True)
 
         # Float representation should be unaltered by truncation
         assert x == math.pi
         # String representations should be correctly formatted
         assert str(x) == '3.14159265358979'
-        assert repr(x) == '"3.14159265358979"'
+        assert repr(x) == repr('3.14159265358979')
 
     def test_auto_format_from_invalid_DS(self):
         """Test truncating floats"""
         # A DSfloat that has a non-valid string representation
-        x = pydicom.valuerep.DSfloat(math.pi)
+        x = DSfloat(math.pi)
 
         # Use this to initialise another with auto_format set to true
-        y = pydicom.valuerep.DSfloat(x, auto_format=True)
+        y = DSfloat(x, auto_format=True)
 
         # Float representation should be unaltered by truncation
         assert y == math.pi
         # String representations should be correctly formatted
         assert str(y) == '3.14159265358979'
-        assert repr(y) == '"3.14159265358979"'
+        assert repr(y) == repr("3.14159265358979")
 
     def test_auto_format_invalid_string(self, enforce_valid_both_fixture):
         """If the user supplies an invalid string, this should be formatted."""
-        x = pydicom.valuerep.DSfloat('3.141592653589793', auto_format=True)
+        x = DSfloat('3.141592653589793', auto_format=True)
 
         # Float representation should be unaltered by truncation
         assert x == float('3.141592653589793')
         # String representations should be correctly formatted
         assert str(x) == '3.14159265358979'
-        assert repr(x) == '"3.14159265358979"'
+        assert repr(x) == repr("3.14159265358979")
 
     def test_auto_format_valid_string(self, enforce_valid_both_fixture):
         """If the user supplies a valid string, this should not be altered."""
-        x = pydicom.valuerep.DSfloat('1.234e-1', auto_format=True)
+        x = DSfloat('1.234e-1', auto_format=True)
 
         # Float representation should be correct
         assert x == 0.1234
         # String representations should be unaltered
         assert str(x) == '1.234e-1'
-        assert repr(x) == '"1.234e-1"'
+        assert repr(x) == repr("1.234e-1")
 
     def test_enforce_valid_values_length(self, enforce_valid_true_fixture):
         """Test that errors are raised when length is too long."""
@@ -589,14 +609,14 @@ class TestDSfloat:
 
     def test_DSfloat_auto_format(self):
         """Test creating a value using DSfloat copies auto_format"""
-        x = pydicom.valuerep.DSfloat(math.pi, auto_format=True)
-        y = pydicom.valuerep.DSfloat(x)
+        x = DSfloat(math.pi, auto_format=True)
+        y = DSfloat(x)
         assert x == y
         assert y == x
         assert y.auto_format
         assert math.pi == y
         assert str(y) == '3.14159265358979'
-        assert repr(y) == '"3.14159265358979"'
+        assert repr(y) == repr("3.14159265358979")
 
     @pytest.mark.parametrize(
         'val',
@@ -606,14 +626,48 @@ class TestDSfloat:
         ]
     )
     def test_enforce_valid_values_value(
-            self,
-            val: Union[float, str],
-            enforce_valid_true_fixture
+        self, val: Union[float, str], enforce_valid_true_fixture
     ):
         """Test that errors are raised when value is invalid."""
         with pytest.raises(ValueError):
             valuerep.DSfloat(val)
 
+    def test_comparison_operators(self):
+        """Tests for the comparison operators"""
+        float_decimal = DSfloat(Decimal(1234.5))
+        for val in (DSfloat("1234.5"), DSfloat(1234.5), float_decimal):
+            assert val == Decimal(1234.5)
+            assert val != Decimal(1235)
+            assert val < Decimal(1235)
+            assert val <= Decimal(1235)
+            assert val > Decimal(1233)
+            assert val >= Decimal(1233)
+
+            assert val == 1234.5
+            assert val != 1235.0
+            assert val < 1235.0
+            assert val <= 1235.0
+            assert val > 1233.0
+            assert val >= 1233.0
+
+            assert val == "1234.5"
+            assert val != " 1234.5"
+            assert val != "1234.50"
+            assert val != "1234.5 "
+            assert val != "1235"
+
+            with pytest.raises(TypeError, match="'<' not supported"):
+                val < "1235"
+
+            with pytest.raises(TypeError, match="'<=' not supported"):
+                val <= "1235"
+
+            with pytest.raises(TypeError, match="'>' not supported"):
+                val > "1233"
+
+            with pytest.raises(TypeError, match="'>=' not supported"):
+                val >= "1233"
+
 
 class TestDSdecimal:
     """Unit tests for pickling DSdecimal"""
@@ -621,7 +675,7 @@ class TestDSdecimal:
         # Check that a pickled DSdecimal is read back properly
         # DSdecimal actually prefers original_string when
         # reading back
-        x = pydicom.valuerep.DSdecimal(19)
+        x = DSdecimal(19)
         x.original_string = "19"
         data1_string = pickle.dumps(x)
         x2 = pickle.loads(data1_string)
@@ -630,21 +684,36 @@ class TestDSdecimal:
 
     def test_float_value(self):
         config.allow_DS_float = False
-        with pytest.raises(
-                TypeError, match="cannot be instantiated with a float value"
-        ):
-            pydicom.valuerep.DSdecimal(9.0)
+        msg = "cannot be instantiated with a float value"
+        with pytest.raises(TypeError, match=msg):
+            DSdecimal(9.0)
         config.allow_DS_float = True
-        assert 9 == pydicom.valuerep.DSdecimal(9.0)
-
-    def test_new_empty_str(self):
-        """Test passing an empty string."""
-        assert pydicom.valuerep.DSdecimal('') is None
+        assert 9 == DSdecimal(9.0)
+
+    def test_new_empty(self):
+        """Test passing an empty value."""
+        assert DSdecimal('') == ''
+        assert DSdecimal('  ') == '  '
+        assert DSdecimal(None) is None
+
+    def test_str_value(self):
+        """Test creating using str"""
+        # Not equal because float(1.2) != Decimal('1.2')
+        assert DSdecimal('1.20') != 1.2
+        assert DSdecimal('1.20') != 1.20
+        # Decimal(1.2) is different to Decimal('1.2')
+        assert DSdecimal('1.20') == Decimal('1.2')
+        assert DSdecimal('1.20') == Decimal('1.20')
+        assert DSdecimal('1.20 ') == Decimal('1.2')
+        assert DSdecimal('1.20 ') == Decimal('1.20')
+        assert DSdecimal('1.20') != '1.2'
+        assert DSdecimal('1.20') == '1.20'
+        assert DSdecimal('1.20 ') == '1.20'
 
     def test_DSfloat(self):
         """Test creating a value using DSfloat."""
-        x = pydicom.valuerep.DSdecimal('1.2345')
-        y = pydicom.valuerep.DSdecimal(x)
+        x = DSdecimal('1.2345')
+        y = DSdecimal(x)
         assert x == y
         assert y == x
         assert Decimal("1.2345") == y
@@ -652,49 +721,49 @@ class TestDSdecimal:
 
     def test_DSdecimal(self):
         """Test creating a value using DSdecimal."""
-        x = pydicom.valuerep.DSfloat('1.2345')
-        y = pydicom.valuerep.DSdecimal(x)
+        x = DSfloat('1.2345')
+        y = DSdecimal(x)
         assert Decimal(1.2345) == y
         assert "1.2345" == y.original_string
 
     def test_repr(self):
         """Test repr(DSdecimal)."""
-        x = pydicom.valuerep.DSdecimal('1.2345')
-        assert '"1.2345"' == repr(x)
+        x = DSdecimal('1.2345')
+        assert repr(x) == repr('1.2345')
 
     def test_auto_format(self, enforce_valid_both_fixture):
         """Test truncating decimal"""
-        x = pydicom.valuerep.DSdecimal(Decimal(math.pi), auto_format=True)
+        x = DSdecimal(Decimal(math.pi), auto_format=True)
 
         # Decimal representation should be unaltered by truncation
         assert x == Decimal(math.pi)
         # String representations should be correctly formatted
         assert str(x) == '3.14159265358979'
-        assert repr(x) == '"3.14159265358979"'
+        assert repr(x) == repr("3.14159265358979")
 
     def test_auto_format_from_invalid_DS(self):
         """Test truncating floats"""
         # A DSdecimal that has a non-valid string representation
-        x = pydicom.valuerep.DSdecimal(math.pi)
+        x = DSdecimal(math.pi)
 
         # Use this to initialise another with auto_format set to true
-        y = pydicom.valuerep.DSdecimal(x, auto_format=True)
+        y = DSdecimal(x, auto_format=True)
 
         # Float representation should be unaltered by truncation
         assert y == math.pi
         # String representations should be correctly formatted
         assert str(y) == '3.14159265358979'
-        assert repr(y) == '"3.14159265358979"'
+        assert repr(y) == repr("3.14159265358979")
 
     def test_auto_format_invalid_string(self, enforce_valid_both_fixture):
         """If the user supplies an invalid string, this should be formatted."""
-        x = pydicom.valuerep.DSdecimal('3.141592653589793', auto_format=True)
+        x = DSdecimal('3.141592653589793', auto_format=True)
 
         # Decimal representation should be unaltered by truncation
         assert x == Decimal('3.141592653589793')
         # String representations should be correctly formatted
         assert str(x) == '3.14159265358979'
-        assert repr(x) == '"3.14159265358979"'
+        assert repr(x) == repr("3.14159265358979")
 
     @pytest.mark.parametrize(
         'val',
@@ -704,9 +773,7 @@ class TestDSdecimal:
         ]
     )
     def test_enforce_valid_values_value(
-            self,
-            val: Union[Decimal, str],
-            enforce_valid_true_fixture
+        self, val: Union[Decimal, str], enforce_valid_true_fixture
     ):
         """Test that errors are raised when value is invalid."""
         with pytest.raises(ValueError):
@@ -714,31 +781,74 @@ class TestDSdecimal:
 
     def test_auto_format_valid_string(self, enforce_valid_both_fixture):
         """If the user supplies a valid string, this should not be altered."""
-        x = pydicom.valuerep.DSdecimal('1.234e-1', auto_format=True)
+        x = DSdecimal('1.234e-1', auto_format=True)
 
         # Decimal representation should be correct
         assert x == Decimal('1.234e-1')
         # String representations should be unaltered
         assert str(x) == '1.234e-1'
-        assert repr(x) == '"1.234e-1"'
+        assert repr(x) == repr("1.234e-1")
 
     def test_DSdecimal_auto_format(self):
         """Test creating a value using DSdecimal copies auto_format"""
-        x = pydicom.valuerep.DSdecimal(math.pi, auto_format=True)
-        y = pydicom.valuerep.DSdecimal(x)
+        x = DSdecimal(math.pi, auto_format=True)
+        y = DSdecimal(x)
         assert x == y
         assert y == x
         assert y.auto_format
         assert math.pi == y
         assert str(y) == '3.14159265358979'
-        assert repr(y) == '"3.14159265358979"'
+        assert repr(y) == repr("3.14159265358979")
+
+    def test_comparison_operators(self):
+        """Tests for the comparison operators"""
+        decimal_decimal = DSdecimal(Decimal(1234.5))
+        for val in (DSdecimal("1234.5"), DSdecimal(1234.5), decimal_decimal):
+            assert val == Decimal(1234.5)
+            assert val != Decimal(1235)
+            assert val < Decimal(1235)
+            assert val <= Decimal(1235)
+            assert val > Decimal(1233)
+            assert val >= Decimal(1233)
+
+            assert val == 1234.5
+            assert val != 1235.0
+            assert val < 1235.0
+            assert val <= 1235.0
+            assert val > 1233.0
+            assert val >= 1233.0
+
+            assert val == "1234.5"
+            assert val != " 1234.5"
+            assert val != "1234.50"
+            assert val != "1234.5 "
+            assert val != "1235"
+
+            with pytest.raises(TypeError, match="'<' not supported"):
+                val < "1235"
+
+            with pytest.raises(TypeError, match="'<=' not supported"):
+                val <= "1235"
+
+            with pytest.raises(TypeError, match="'>' not supported"):
+                val > "1233"
+
+            with pytest.raises(TypeError, match="'>=' not supported"):
+                val >= "1233"
 
 
 class TestIS:
     """Unit tests for IS"""
     def test_empty_value(self):
         assert IS(None) is None
-        assert IS("") is None
+        assert IS("") == ""
+        assert IS("  ") == "  "
+
+    def test_str_value(self):
+        """Test creating using str"""
+        assert IS('1') == 1
+        assert IS('1 ') == 1
+        assert IS(' 1 ') == 1
 
     def test_valid_value(self):
         assert 42 == IS(42)
@@ -756,7 +866,7 @@ class TestIS:
 
     def test_pickling(self):
         # Check that a pickled IS is read back properly
-        x = pydicom.valuerep.IS(921)
+        x = IS(921)
         x.original_string = "hello"
         data1_string = pickle.dumps(x)
         x2 = pickle.loads(data1_string)
@@ -766,7 +876,7 @@ class TestIS:
     def test_longint(self, allow_invalid_values):
         # Check that a long int is read properly
         # Will not work with enforce_valid_values
-        x = pydicom.valuerep.IS(3103050000)
+        x = IS(3103050000)
         data1_string = pickle.dumps(x)
         x2 = pickle.loads(data1_string)
         assert x.real == x2.real
@@ -778,29 +888,65 @@ class TestIS:
             r"to override the value check"
         )
         with pytest.raises(OverflowError, match=msg):
-            pydicom.valuerep.IS(3103050000)
+            IS(3103050000)
 
     def test_str(self):
         """Test IS.__str__()."""
-        val = pydicom.valuerep.IS(1)
-        assert "1" == str(val)
+        val = IS(1)
+        assert str(val) == '1'
 
-        val = pydicom.valuerep.IS("1")
-        assert "1" == str(val)
+        val = IS("1")
+        assert str(val) == '1'
 
-        val = pydicom.valuerep.IS("1.0")
-        assert "1.0" == str(val)
+        val = IS("1.0")
+        assert str(val) == '1.0'
 
     def test_repr(self):
         """Test IS.__repr__()."""
-        val = pydicom.valuerep.IS(1)
-        assert '"1"' == repr(val)
+        val = IS(1)
+        assert repr(val) == repr('1')
+
+        val = IS("1")
+        assert repr(val) == repr('1')
+        assert repr(val) == repr("1")
+
+        val = IS("1.0")
+        assert str(val) == '1.0'
+
+    def test_comparison_operators(self):
+        """Tests for the comparison operators"""
+        for val in (IS("1234"), IS(1234), IS(1234.0)):
+            assert val == 1234
+            assert val != 1235
+            assert val < 1235
+            assert val <= 1235
+            assert val > 1233
+            assert val >= 1233
 
-        val = pydicom.valuerep.IS("1")
-        assert '"1"' == repr(val)
+            assert val == 1234.0
+            assert val != 1235.0
+            assert val < 1235.0
+            assert val <= 1235.0
+            assert val > 1233.0
+            assert val >= 1233.0
 
-        val = pydicom.valuerep.IS("1.0")
-        assert "1.0" == str(val)
+            assert val == "1234"
+            assert val != "1234.0"
+            assert val != "1235"
+            assert val != "1234 "
+            assert val != " 1234"
+
+            with pytest.raises(TypeError, match="'<' not supported"):
+                val < "1235"
+
+            with pytest.raises(TypeError, match="'<=' not supported"):
+                val <= "1235"
+
+            with pytest.raises(TypeError, match="'>' not supported"):
+                val > "1233"
+
+            with pytest.raises(TypeError, match="'>=' not supported"):
+                val >= "1233"
 
 
 class TestBadValueRead:
@@ -1304,3 +1450,84 @@ def test_person_name_unicode_warns():
             from pydicom.valuerep import PersonNameUnicode
 
     assert PersonNameUnicode == PersonName
+
+
+VALUE_REFERENCE = [
+    # (VR, Python setter type, (VM 0 values), (VM >= 1 values), keyword)
+    ("AE", str, (None, ""), ("foo", "bar"), 'Receiver'),
+    ("AS", str, (None, ""), ("foo", "bar"), 'PatientAge'),
+    ("AT", int, (None, ), (0, 2**32 - 1), 'OffendingElement'),
+    ("CS", str, (None, ""), ("foo", "bar"), 'QualityControlSubject'),
+    ("DA", str, (None, ""), ("20010203", "20020304"), 'PatientBirthDate'),
+    ("DS", str, (None, ""), ("-1.5", "3.2"), 'PatientWeight'),
+    ("DS", int, (None, ""), (-1, 3), 'PatientWeight'),
+    ("DS", float, (None, ""), (-1.5, 3.2), 'PatientWeight'),
+    ("DT", str, (None, ""), ("20010203040506", "2000"), 'AcquisitionDateTime'),
+    ("FD", float, (None, ), (-1.5, 3.2), 'RealWorldValueLUTData'),
+    ("FL", float, (None, ), (-1.5, 3.2), 'VectorAccuracy'),
+    ("IS", str, (None, ""), ("0", "25"), 'BeamNumber'),
+    ("IS", int, (None, ""), (0, 25), 'BeamNumber'),
+    ("IS", float, (None, ""), (0.0, 25.0), 'BeamNumber'),
+    ("LO", str, (None, ""), ("foo", "bar"), 'DataSetSubtype'),
+    ("LT", str, (None, ""), ("foo", "bar"), 'ExtendedCodeMeaning'),
+    ("OB", bytes, (None, b""), (b"\x00\x01", ), 'FillPattern'),
+    ("OD", bytes, (None, b""), (b"\x00\x01", ), 'DoubleFloatPixelData'),
+    ("OF", bytes, (None, b""), (b"\x00\x01", ), 'UValueData'),
+    ("OL", bytes, (None, b""), (b"\x00\x01", ), 'TrackPointIndexList'),
+    ("OV", bytes, (None, b""), (b"\x00\x01", ), 'SelectorOVValue'),
+    ("OW", bytes, (None, b""), (b"\x00\x01", ), 'TrianglePointIndexList'),
+    ("PN", str, (None, ""), ("foo", "bar"), 'PatientName'),
+    ("SH", str, (None, ""), ("foo", "bar"), 'CodeValue'),
+    ("SL", int, (None, ), (-2**31, 2**31 - 1), 'RationalNumeratorValue'),
+    ("SQ", list, ([], ), (Dataset(), Dataset()), 'BeamSequence'),
+    ("SS", int, (None, ), (-2**15, 2**15 - 1), 'SelectorSSValue'),
+    ("ST", str, (None, ""), ("foo", "bar"), 'InstitutionAddress'),
+    ("SV", int, (None, ), (-2**63, 2**63 - 1), 'SelectorSVValue'),
+    ("TM", str, (None, ""), ("123456", "000000"), 'StudyTime'),
+    ("UC", str, (None, ""), ("foo", "bar"), 'LongCodeValue'),
+    ("UI", str, (None, ""), ("foo", "bar"), 'SOPClassUID'),
+    ("UL", int, (None, ), (0, 2**32 - 1), 'SimpleFrameList'),
+    ("UN", bytes, (None, b""), (b"\x00\x01", ), 'SelectorUNValue'),
+    ("UR", str, (None, ""), ("foo", "bar"), 'CodingSchemeURL'),
+    ("US", int, (None, ), (0, 2**16 - 1), 'SourceAcquisitionBeamNumber'),
+    ("UT", str, (None, ""), ("foo", "bar"), 'StrainAdditionalInformation'),
+    ("UV", int, (None, ), (0, 2**64 - 1), 'SelectorUVValue')
+]
+
+
+@pytest.mark.parametrize("vr, pytype, vm0, vmN, keyword", VALUE_REFERENCE)
+def test_set_value(vr, pytype, vm0, vmN, keyword):
+    """Test that element values are set consistently"""
+    # Test VM = 0
+    ds = Dataset()
+    for value in vm0:
+        setattr(ds, keyword, value)
+        elem = ds[keyword]
+        assert elem.VR == vr
+        assert elem.value == value
+        assert value == elem.value
+
+    # Test VM = 1
+    ds = Dataset()
+    value = vmN[0]
+    if vr == 'SQ':
+        setattr(ds, keyword, [value])
+        elem = ds[keyword]
+        assert elem.value[0] == value
+        assert value == elem.value[0]
+    else:
+        setattr(ds, keyword, value)
+        elem = ds[keyword]
+        assert elem.value == value
+        assert value == elem.value
+
+    if vr[0] == 'O':
+        return
+
+    # Test VM > 1
+    ds = Dataset()
+    value = vmN[0]
+    setattr(ds, keyword, list(vmN))
+    elem = ds[keyword]
+    assert elem.value == list(vmN)
+    assert list(vmN) == elem.value