Repository: planetarypy/pvl

Issue:
Improved collections handling proposal
`pvl` 0.x defines two fundamentally new classes: `OrderedMultiDict` and `Units` (and there are thin wrappers around OrderedMultiDict that users are more familiar with: `PVLModule`, `PVLGroup`, `PVLObject`, etc.) in `_collections.py`.

Initial 1.0.0-alpha work did not alter this module much, nor these classes.  Since the `OrderedMultiDict` as `PVLModule` is the primary return from the `pvl` loaders, messing with it has the potential for disruption.  The changes I am proposing here should not change anything, they simply define better interfaces for the library to use internally, the loaders still return the old `pvl` OrderedMultiDict.

However, the work to implement 3rd party quantity objects and the discussions in #52 have made me think about the "interface" we provide, and what operations under the hood expect from various objects.

I think making the following changes will improve our codebase and also make things more logical to external users without breaking things:

#### 1. Change `_collections.py` to `collections.py`

The truth is that since we are returning an OrderedMultiDict as a PVLModule object through the loaders, and including Units objects within that structure, the classes and interfaces that are in this module really aren't purely "internal" and so the module should not begin with an underbar, which typically signifies that.  This should have no impact, as external users shouldn't have been using _collections by name as the objects were made available under the top-level pvl namespace, and we should continue to do that.

#### 2. Rename the `Units` class to `Quantity`

My work with the 3rd party libraries made it clear to me that this class that `pvl` 0.x named `Units`, isn't just a 'units' class, it is a combined thing that contains both a value and a units parameter.  Other libraries name this kind of thing a Quantity class, which is what I think this is.  So I think we should define `Quantity` (identical to how `Units` is defined), and use it consistently through our library.  The `Units` class would remain with a deprecation warning.

#### 3. Define a new abstract base class: `MutableMappingSequence`

The `pvl` OrderedMultiDict implements both a dict-like Mapping interface, and a list-like Sequence interface. In order to potentially diversify away from `pvl` defining and maintaining its own OrderedMultiDict at some point in the future, we should define an abstract base class that satisfies our needs (which would just formalize the interface to the already-working OrderedMultiDict), and then genericize the internal workings of the library to deal with classes and objects that satisfy this duck-typed interface.  Since it is literally just an object that implements those two interfaces, it is trivial to define:
```
class MutableMappingSequence(
    MutableMapping, MutableSequence
):
    pass
```

I don't know that I need to go into the benefits of defining an ABC, but this would allow us to be more specific within the library about the kind of object we need to parse/decode and encode PVL text, and would allow alternate objects (as long as they satisfy the interface) to be used.

Discussion:

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index de9ec10..caffb43 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,6 +3,13 @@
 History
 -------
 
+1.0.0-alpha.8 (2020-08-01)
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Renamed the _collections module to just collections.
+* Renamed the Units class to Quantity (Units remains, but has a deprecation warning).
+* Defined a new ABC: pvl.collections.MutableMappingSequence
+* More detail for these changes can be found in Issue #62.
+
 1.0.0-alpha.7 (2020-07-29)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Created a new exceptions.py module and grouped all pvl Exceptions
diff --git a/docs/parsing.rst b/docs/parsing.rst
index dc3217b..8805747 100644
--- a/docs/parsing.rst
+++ b/docs/parsing.rst
@@ -255,6 +255,6 @@ This is very similar to parsing PVL text from a file, but you use
  >>> import pvl
  >>> url = 'https://hirise-pds.lpl.arizona.edu/PDS/RDR/ESP/ORB_017100_017199/ESP_017173_1715/ESP_017173_1715_RED.LBL'
  >>> pvl.loadu(url)['VIEWING_PARAMETERS']['PHASE_ANGLE']
- Units(value=50.784875, units='DEG')
+ Quantity(value=50.784875, units='DEG')
 
 Of course, other kinds of URLs, like file, ftp, rsync, sftp and more can be used.
diff --git a/docs/quantities.rst b/docs/quantities.rst
index 92d262c..1ae2f19 100644
--- a/docs/quantities.rst
+++ b/docs/quantities.rst
@@ -12,7 +12,7 @@ and the units of that value. However, libraries like ``astropy``
 and ``pint`` have implemented "quantity" objects (and managed to
 name them both Quantity, but they have slightly different interfaces).
 In order to avoid optional dependencies, the ``pvl`` library provides
-the :class:`pvl._collections.Units` class, implemented as a
+the :class:`pvl.collections.Quantity` class, implemented as a
 :class:`collections.namedtuple` with a ``value`` and a ``unit``
 parameter.  However, the ``unit`` parameter is just a string and
 so the ``pvl`` quantity objects doesn't have the super-powers that
@@ -20,11 +20,11 @@ the ``astropy`` and ``pint`` quntity objects do.
 
 By default, this means that when PVL text is parsed by :func:`pvl.load`
 or :func:`pvl.loads` and when a PVL Value followed by a PVL Units
-Expression is encountered, a :class:`pvl._collections.Units` object
+Expression is encountered, a :class:`pvl.collections.Quantity` object
 will be placed in the returned dict-like.
 
 Likewise when :func:`pvl.dump` or :func:`pvl.dumps` encounters a
-:class:`pvl._collections.Units` its value and units will be serialized
+:class:`pvl.collections.Quantity` its value and units will be serialized
 with the right PVL syntax.
 
 However, the ``pvl`` library also supports the use of other quantity
@@ -61,7 +61,7 @@ In order to get the encoding side of the ``pvl`` library to write out the
 correct kind of PVL text based on some quantity object is more difficult 
 due to the wide variety of ways that quantity objects are written in 3rd 
 party libaries.  At this time, the ``pvl`` library can properly encode
-:class:`pvl._collecitons.Units`, :class:`astropy.units.Quantity`, and
+:class:`pvl.collecitons.Quantity`, :class:`astropy.units.Quantity`, and
 :class:`pint.Quantity` objects (or objects that pass an ``isinstance()``
 test for those objects).  Any other kind of quantity object in the 
 data structure passed to :func:`pvl.dump` or :func:`pvl.dumps` will
@@ -107,9 +107,9 @@ an example::
  >>> pvl_text = "length = 42 <m/s>"
  >>> regular = pvl.loads(pvl_text)
  >>> print(regular['length'])
- Units(value=42, units='m/s')
+ Quantity(value=42, units='m/s')
  >>> print(type(regular['length']))
- <class 'pvl._collections.Units'>
+ <class 'pvl.collections.Quantity'>
 
  >>> from pvl.decoder import OmniDecoder
  >>> from astropy import units as u
diff --git a/pvl/__init__.py b/pvl/__init__.py
index a4ec452..b6a5fd7 100755
--- a/pvl/__init__.py
+++ b/pvl/__init__.py
@@ -14,16 +14,17 @@ from pathlib import Path
 
 from .encoder import PDSLabelEncoder, PVLEncoder
 from .parser import PVLParser, OmniParser
-from ._collections import (
+from .collections import (
     PVLModule,
     PVLGroup,
     PVLObject,
+    Quantity,
     Units,
 )
 
 __author__ = 'The pvl Developers'
 __email__ = 'trevor@heytrevor.com'
-__version__ = '1.0.0-alpha.7'
+__version__ = '1.0.0-alpha.8'
 __all__ = [
     'load',
     'loads',
@@ -32,6 +33,7 @@ __all__ = [
     'PVLModule',
     'PVLGroup',
     'PVLObject',
+    'Quantity',
     'Units',
 ]
 
@@ -71,13 +73,13 @@ def get_text_from(path) -> str:
         # fails.  So open the file as a bytestream, and read until
         # we can't decode.  We don't want to just run the .read_bytes()
         # method of Path, because this could be a giant file.
-        with open(p, mode='rb') as f:
+        with open(path, mode='rb') as f:
             return decode_by_char(f)
     except TypeError:
         # Not an os.PathLike, maybe it is an already-opened file object
         if path.readable():
+            position = path.tell()
             try:
-                position = path.tell()
                 s = path.read()
                 if isinstance(s, bytes):
                     # Oh, it was opened in 'b' mode, need to rewind and
diff --git a/pvl/_collections.py b/pvl/collections.py
similarity index 57%
rename from pvl/_collections.py
rename to pvl/collections.py
index b724023..bf293dc 100644
--- a/pvl/_collections.py
+++ b/pvl/collections.py
@@ -1,4 +1,23 @@
 # -*- coding: utf-8 -*-
+"""Parameter Value Language container datatypes providing enhancements
+to Python general purpose built-in containers.
+
+To enable efficient operations on parsed PVL text, we need an object
+that acts as both a dict-like Mapping container and a list-like
+Sequence container, essentially an ordered multi-dict.  There is
+no existing object or even an Abstract Base Class in the Python
+Standard Library for such an object.  So we define the
+MutableMappingSequence ABC here, which is (as the name implies) an
+abstract base class that implements both the Python MutableMapping
+and Mutable Sequence ABCs. We also provide an implementation, the
+OrderedMultiDict.
+
+Additionally, for PVL Values which also have an associated PVL Units
+Expression, they need to be returned as a quantity object which contains
+both a notion of a value and the units for that value.  Again, there
+is no fundamental Python type for a quantity, so we define the Quantity
+class (formerly the Units class).
+"""
 # Copyright 2015, 2017, 2019-2020, ``pvl`` library authors.
 #
 # Reuse is permitted under the terms of the license.
@@ -6,8 +25,31 @@
 # top level of this library.
 
 import pprint
-from collections import namedtuple
-from collections.abc import Mapping, MutableMapping
+import warnings
+from abc import abstractmethod
+from collections import namedtuple, abc
+
+
+class MutableMappingSequence(
+    abc.MutableMapping, abc.MutableSequence
+):
+    """ABC for a mutable object that has both mapping and
+    sequence characteristics.
+
+    Must implement `.getall()` and `.popall()` since MutableMappingSequence
+    can have many values for a single key, while `.get(k)` and
+    `.pop(k)` return and operate on a single value, the *all*
+    versions return and operate on all values in the MutableMappingSequence
+    with the key `k`.
+    """
+
+    @abstractmethod
+    def getall(self, key):
+        pass
+
+    @abstractmethod
+    def popall(self, key):
+        pass
 
 
 dict_setitem = dict.__setitem__
@@ -92,7 +134,7 @@ class ValuesView(MappingView):
         return values.index(value)
 
 
-class OrderedMultiDict(dict, MutableMapping):
+class OrderedMultiDict(dict, MutableMappingSequence):
     """A ``dict`` like container.
 
     This container preserves the original ordering as well as
@@ -173,9 +215,8 @@ class OrderedMultiDict(dict, MutableMapping):
 
         return "{!s}([\n{!s}\n])".format(type(self).__name__, '\n'.join(lines))
 
-    get = MutableMapping.get
-    update = MutableMapping.update
-    pop = MutableMapping.pop
+    get = abc.MutableMapping.get
+    update = abc.MutableMapping.update
 
     def keys(self):
         return KeysView(self)
@@ -214,7 +255,7 @@ class OrderedMultiDict(dict, MutableMapping):
 
         iterable = args[0] if args else None
         if iterable:
-            if isinstance(iterable, Mapping) or hasattr(iterable, 'items'):
+            if isinstance(iterable, abc.Mapping) or hasattr(iterable, 'items'):
                 for key, value in iterable.items():
                     self.append(key, value)
             elif hasattr(iterable, 'keys'):
@@ -227,15 +268,48 @@ class OrderedMultiDict(dict, MutableMapping):
         for key, value in kwargs.items():
             self.append(key, value)
 
-    def getlist(self, key):
+    def getall(self, key) -> abc.Sequence:
+        """Returns a list of all the values for a named field.
+        Returns KeyError if the key doesn't exist.
+        """
+        return list(dict_getitem(self, key))
+
+    def getlist(self, key) -> abc.Sequence:
         """Returns a list of all the values for the named field.
         Returns an empty list if the key doesn't exist.
         """
+        warnings.warn(
+            "The pvl.collections.OrderedMultiDict.getlist(k) function is "
+            "deprecated in favor of .getall(), please begin using it, as "
+            ".getlist() may be removed in the next major patch.",
+            PendingDeprecationWarning
+        )
+
         try:
-            return list(dict_getitem(self, key))
+            return self.getall(key)
         except KeyError:
             return []
 
+    # Turns out that this super-class function, even though it doesn't have
+    # a concept of multiple keys, clears out multiple elements with the key,
+    # probably because of how __delitem__ is defined:
+    popall = abc.MutableMapping.pop
+
+    def pop(self, *args, **kwargs):
+        """Removes all items with the specified *key*."""
+
+        warnings.warn(
+            "The pvl.collections.OrderedMultiDict.pop(k) function removes "
+            "all keys with value k to be backwards compatible with the "
+            "pvl 0.x architecture, despite the new concept in "
+            "pvl.collections.MutableMappingSequence, this concept of "
+            "operations for .pop(k) may change in future versions."
+            "Consider using .popall(k) instead.",
+            FutureWarning
+        )
+
+        return self.popall(*args, *kwargs)
+
     def popitem(self):
         if not self:
             raise KeyError('popitem(): {!s} '.format(type(self).__name__) +
@@ -253,6 +327,45 @@ class OrderedMultiDict(dict, MutableMapping):
     def copy(self):
         return type(self)(self)
 
+    def insert(self, index: int, *args) -> None:
+        """Inserts at the index given by *index*.
+
+        The first positional argument will be taken as the
+        *index*. If three arguments are given, the second will be taken
+        as the *key*, and the third as the *value*.  If only two arguments are
+        given, the second must be a two-element sequence, where the first will
+        be the *key* and the second the *value*.
+        """
+        if len(args) == 1:
+            if len(args[0]) == 2:
+                key, value = args[0]
+            else:
+                raise IndexError(
+                    "If a sequence is provided to the second positional "
+                    f"argument of pvl.OrderedMultiDict.insert() it must have "
+                    f"exactly 2 elements, but it is {args[0]}"
+                )
+        elif len(args) == 2:
+            self.__items.insert(index, args)
+            key, value = args
+        else:
+            raise TypeError(
+                f"{self.__name__}.insert() takes 2 or 3 positional arguments, "
+                f"but {len(args)} were given."
+            )
+
+        self.__items.insert(index, (key, value))
+
+        # Make sure indexing works with the new item
+        if key in self:
+            value_list = [val for k, val in self.__items if
+                          k == key]
+            dict_setitem(self, key, value_list)
+        else:
+            dict_setitem(self, key, [value])
+
+        return
+
     def __insert_wrapper(func):
         """Make sure the arguments given to the insert methods are correct."""
 
@@ -266,10 +379,10 @@ class OrderedMultiDict(dict, MutableMapping):
             return func(self, key, new_item, instance)
         return check_func
 
-    def _get_index_for_insert(self, key, instance):
+    def _get_index_for_insert(self, key, instance: int) -> int:
         """Get the index of the key to insert before or after."""
         if instance == 0:
-            # Index method will return the first occurence of the key
+            # Index method will return the first occurrence of the key
             index = self.keys().index(key)
         else:
             occurrence = -1
@@ -277,37 +390,38 @@ class OrderedMultiDict(dict, MutableMapping):
                 if k == key:
                     occurrence += 1
                     if occurrence == instance:
-                        # Found the key and the correct occurence of the key
+                        # Found the key and the correct occurrence of the key
                         break
 
             if occurrence != instance:
                 # Gone through the entire list of keys and the instance number
-                # given is too high for the number of occurences of the key
+                # given is too high for the number of occurrences of the key
                 raise ValueError(f"Cannot insert before/after the {instance} "
                                  f"instance of the key '{key}' since there are "
-                                 f"only {occurrence} occurences of the key")
+                                 f"only {occurrence} occurrences of the key")
         return index
 
-    def _insert_item(self, key, new_item, instance, is_after):
+    def _insert_item(
+            self, key, new_item: abc.Iterable, instance: int, is_after: bool
+    ):
         """Insert a new item before or after another item."""
         index = self._get_index_for_insert(key, instance)
         index = index + 1 if is_after else index
-        self.__items = self.__items[:index] + new_item + self.__items[index:]
-        # Make sure indexing works with new items
-        for new_key, new_value in new_item:
-            if new_key in self:
-                value_list = [val for k, val in self.__items if k == new_key]
-                dict_setitem(self, new_key, value_list)
-            else:
-                dict_setitem(self, new_key, [new_value])
+
+        # But new_item is always a list of two-tuples, even if only one, and
+        # all should be inserted, so despite the singular "an item"
+        # in the doc strings, this could be a whole bunch.
+        for pair in new_item:
+            self.insert(index, pair)
+            index += 1
 
     @__insert_wrapper
-    def insert_after(self, key, new_item, instance=0):
+    def insert_after(self, key, new_item: abc.Iterable, instance=0):
         """Insert an item after a key"""
         self._insert_item(key, new_item, instance, True)
 
     @__insert_wrapper
-    def insert_before(self, key, new_item, instance=0):
+    def insert_before(self, key, new_item: abc.Iterable, instance=0):
         """Insert an item before a key"""
         self._insert_item(key, new_item, instance, False)
 
@@ -335,5 +449,22 @@ class PVLObject(PVLAggregation):
     pass
 
 
-class Units(namedtuple('Units', ['value', 'units'])):
+class Quantity(namedtuple('Quantity', ['value', 'units'])):
+    """A simple collections.namedtuple object to contain
+    a value and units parameter.
+
+    If you need more comprehensive units handling, you
+    may want to use the astropy.units.Quantity object,
+    the pint.Quantity object, or some other 3rd party
+    object.  Please see the documentation on :doc:`quantities`
+    for how to use 3rd party Quantity objects with pvl.
+    """
     pass
+
+
+class Units(Quantity):
+    warnings.warn(
+        "The pvl.collections.Units object is deprecated, and may be removed at "
+        "the next major patch. Please use pvl.collections.Quantity instead.",
+        PendingDeprecationWarning
+    )
diff --git a/pvl/decoder.py b/pvl/decoder.py
index 118a182..1894aba 100644
--- a/pvl/decoder.py
+++ b/pvl/decoder.py
@@ -21,7 +21,7 @@ from itertools import repeat, chain
 from warnings import warn
 
 from .grammar import PVLGrammar, ODLGrammar
-from ._collections import Units
+from .collections import Quantity
 from .exceptions import QuantityError
 
 
@@ -53,9 +53,9 @@ class PVLDecoder(object):
     :param grammar: defaults to a :class:`pvl.grammar.PVLGrammar`, but can
         be any object that implements the :class:`pvl.grammar` interface.
 
-    :param quantity_cls: defaults to :class:`pvl._collections.Units`, but
+    :param quantity_cls: defaults to :class:`pvl.collections.Quantity`, but
         could be any class object that takes two arguments, where the
-        first is the value, and the second is the units type.
+        first is the value, and the second is the units value.
     """
 
     def __init__(self, grammar=None, quantity_cls=None):
@@ -69,7 +69,7 @@ class PVLDecoder(object):
             raise Exception
 
         if quantity_cls is None:
-            self.quantity_cls = Units
+            self.quantity_cls = Quantity
         else:
             self.quantity_cls = quantity_cls
 
diff --git a/pvl/encoder.py b/pvl/encoder.py
index f1ba1d4..d8e5a0c 100644
--- a/pvl/encoder.py
+++ b/pvl/encoder.py
@@ -15,15 +15,24 @@ import datetime
 import re
 import textwrap
 
-from collections import abc
+from collections import abc, namedtuple
 from warnings import warn
 
-from ._collections import PVLObject, PVLGroup, Units
+from .collections import PVLObject, PVLGroup, Quantity
 from .grammar import PVLGrammar, ODLGrammar, ISISGrammar
 from .token import Token
 from .decoder import PVLDecoder, ODLDecoder
 
 
+class QuantTup(namedtuple('QuantTup', ['cls', 'value_prop', 'units_prop'])):
+    """
+    This class is just a convenient namedtuple for internally keeping track
+    of quantity classes that encoders can deal with.  In general, users
+    should not be instantiating this, instead use your encoder's
+    add_quantity_cls() function.
+    """
+
+
 class PVLEncoder(object):
     """An encoder based on the rules in the CCSDS-641.0-B-2 'Blue Book'
     which defines the PVL language.
@@ -47,13 +56,24 @@ class PVLEncoder(object):
         it won't.  Defaults to True.
     :param newline: is the string that will be placed at the end of each
         'line' of output (and counts against *width*), defaults to '\\\\n'.
+    :param group_class: must this class will be tested against with
+        isinstance() to determine if various elements of the dict-like
+        passed to encode() should be encoded as a PVL Group or PVL Object,
+        defaults to PVLGroup.
+    :param object_class: must be a class that can take a *group_class*
+        object in its constructor (essentially converting a *group_class*
+        to an *object_class*), otherwise will raise TypeError.  Defaults
+        to PVLObject.
     """
 
     def __init__(self, grammar=None, decoder=None,
                  indent: int = 2, width: int = 80,
                  aggregation_end: bool = True,
                  end_delimiter: bool = True,
-                 newline: str = '\n'):
+                 newline: str = '\n',
+                 group_class=PVLGroup,
+                 object_class=PVLObject
+                 ):
 
         if grammar is None:
             if decoder is not None:
@@ -80,23 +100,48 @@ class PVLEncoder(object):
 
         # This list of 3-tuples *always* has our own pvl quantity object,
         # and should *only* be added to with self.add_quantity_cls().
-        self.quantities = [(Units, 'value', 'units')]
+        self.quantities = [QuantTup(Quantity, 'value', 'units')]
+        self._import_quantities()
+
+        if issubclass(group_class, abc.Mapping):
+            self.grpcls = group_class
+        else:
+            raise TypeError("The group_class must be a Mapping type.")
 
+        if issubclass(object_class, abc.Mapping):
+            self.objcls = object_class
+        else:
+            raise TypeError('The object_class must be a Mapping type.')
+
+        try:
+            self.objcls(self.grpcls())
+        except TypeError:
+            raise TypeError(
+                f"The object_class type ({object_class}) cannot be "
+                f"instantiated with an argument that is of type "
+                f"group_class ({group_class})."
+            )
+
+    def _import_quantities(self):
+        warn_str = ("The {} library is not present, so {} objects will "
+                    "not be properly encoded.")
         try:
             from astropy import units as u
             self.add_quantity_cls(u.Quantity, 'value', 'unit')
         except ImportError:
-            warn("The astropy library is not present, so "
-                 "astropy.units.Quantity objects will not "
-                 "be properly encoded.", ImportWarning)
+            warn(
+                warn_str.format("astropy", "astropy.units.Quantity"),
+                ImportWarning
+            )
 
         try:
             from pint import Quantity as q
             self.add_quantity_cls(q, 'magnitude', 'units')
         except ImportError:
-            warn("The pint library is not present, so "
-                 "pint.Quantity objects will not "
-                 "be properly encoded.", ImportWarning)
+            warn(
+                warn_str.format("pint", "pint.Quantity"),
+                ImportWarning
+            )
 
     def add_quantity_cls(self, cls, value_prop: str, units_prop: str):
         """Adds a quantity class to the list of possible
@@ -122,7 +167,7 @@ class PVLEncoder(object):
                 raise AttributeError(f"The class ({cls}) does not have an "
                                      f" attribute named {prop}.")
 
-        self.quantities.append((cls, value_prop, units_prop))
+        self.quantities.append(QuantTup(cls, value_prop, units_prop))
 
     def format(self, s: str, level: int = 0) -> str:
         """Returns a string derived from *s*, which
@@ -150,7 +195,7 @@ class PVLEncoder(object):
         else:
             return prefix + s
 
-    def encode(self, module: dict) -> str:
+    def encode(self, module: abc.Mapping) -> str:
         """Returns a ``str`` formatted as a PVL document based
         on the dict-like *module* object
         according to the rules of this encoder.
@@ -171,12 +216,12 @@ class PVLEncoder(object):
             if not self.grammar.char_allowed(c):
                 raise ValueError('Encountered a character that was not '
                                  'a valid character according to the '
-                                 f'grammar: "{c}", it is in: '
-                                 '"{}"'.format(s[i - 5, i + 5]))
+                                 'grammar: "{}", it is in: '
+                                 '"{}"'.format(c, s[i - 5, i + 5]))
 
         return self.newline.join(lines)
 
-    def encode_module(self, module: dict, level: int = 0) -> str:
+    def encode_module(self, module: abc.Mapping, level: int = 0) -> str:
         """Returns a ``str`` formatted as a PVL module based
         on the dict-like *module* object according to the
         rules of this encoder, with an indentation level
@@ -189,19 +234,19 @@ class PVLEncoder(object):
 
         non_agg_key_lengths = list()
         for k, v in module.items():
-            if not isinstance(v, dict):
+            if not isinstance(v, abc.Mapping):
                 non_agg_key_lengths.append(len(k))
         longest_key_len = max(non_agg_key_lengths, default=0)
 
         for k, v in module.items():
-            if isinstance(v, dict):
+            if isinstance(v, abc.Mapping):
                 lines.append(self.encode_aggregation_block(k, v, level))
             else:
                 lines.append(self.encode_assignment(k, v, level,
                                                     longest_key_len))
         return self.newline.join(lines)
 
-    def encode_aggregation_block(self, key: str, value: dict,
+    def encode_aggregation_block(self, key: str, value: abc.Mapping,
                                  level: int = 0) -> str:
         """Returns a ``str`` formatted as a PVL Aggregation Block with
         *key* as its name, and its contents based on the
@@ -211,10 +256,9 @@ class PVLEncoder(object):
         """
         lines = list()
 
-        agg_keywords = None
-        if isinstance(value, PVLGroup):
+        if isinstance(value, self.grpcls):
             agg_keywords = self.grammar.group_pref_keywords
-        elif isinstance(value, dict):
+        elif isinstance(value, abc.Mapping):
             agg_keywords = self.grammar.object_pref_keywords
         else:
             raise ValueError('The value {value} is not dict-like.')
@@ -458,7 +502,7 @@ class ODLEncoder(PVLEncoder):
         super().__init__(grammar, decoder, indent, width, aggregation_end,
                          end_delimiter, newline)
 
-    def encode(self, module: dict) -> str:
+    def encode(self, module: abc.Mapping) -> str:
         """Extends parent function, but ODL requires that there must be
         a spacing or format character after the END statement and this
         adds the encoder's ``newline`` sequence.
@@ -466,8 +510,7 @@ class ODLEncoder(PVLEncoder):
         s = super().encode(module)
         return s + self.newline
 
-    @staticmethod
-    def is_scalar(value) -> bool:
+    def is_scalar(self, value) -> bool:
         """Returns a boolean indicating whether the *value* object
         qualifies as an ODL 'scalar_value'.
 
@@ -476,18 +519,22 @@ class ODLEncoder(PVLEncoder):
 
         For Python, these correspond to the following:
 
-        * numeric_value: int, float, and Units() whose value
+        * numeric_value: int, float, and Quantity whose value
           is int or float
         * date_time_string: datetime objects
         * text_string_value: str
         * symbol_value: str
 
         """
-        if isinstance(value, Units):
-            if isinstance(value.value, (int, float)):
-                return True
-        elif isinstance(value, (int, float, datetime.date, datetime.datetime,
-                                datetime.time, str)):
+        for quant in self.quantities:
+            if isinstance(value, quant.cls):
+                if isinstance(getattr(value, quant.value_prop), (int, float)):
+                    return True
+
+        if isinstance(
+                value, (int, float, datetime.date, datetime.datetime,
+                        datetime.time, str)
+        ):
             return True
 
         return False
@@ -518,10 +565,11 @@ class ODLEncoder(PVLEncoder):
         else:
             return False
 
-    def is_identifier(self, value):
+    @staticmethod
+    def is_identifier(value):
         """Returns true if *value* is an ODL Identifier, false otherwise.
 
-        An ODL Identifier is compsed of letters, digits, and underscores.
+        An ODL Identifier is composed of letters, digits, and underscores.
         The first character must be a letter, and the last must not
         be an underscore.
         """
@@ -548,8 +596,6 @@ class ODLEncoder(PVLEncoder):
         else:
             return False
 
-        return False
-
     def needs_quotes(self, s: str) -> bool:
         """Return true if *s* is an ODL Identifier, false otherwise.
 
@@ -587,7 +633,6 @@ class ODLEncoder(PVLEncoder):
             raise ValueError('ODL keywords must be 30 characters or less '
                              f'in length, this one is longer: {key}')
 
-        ident = ''
         if((key.startswith('^') and self.is_assignment_statement(key[1:]))
            or self.is_assignment_statement(key)):
             ident = key.upper()
@@ -595,8 +640,7 @@ class ODLEncoder(PVLEncoder):
             raise ValueError(f'The keyword "{key}" is not a valid ODL '
                              'Identifier.')
 
-        s = ''
-        s += '{} = '.format(ident.ljust(key_len))
+        s = '{} = '.format(ident.ljust(key_len))
         s += self.encode_value(value)
 
         if self.end_delimiter:
@@ -643,10 +687,15 @@ class ODLEncoder(PVLEncoder):
         """Extends parent function by only allowing Units Expressions for
         numeric values.
         """
-        if(isinstance(value, Units) and
-           not isinstance(value.value, (int, float))):
-            raise ValueError('Unit expressions are only allowed '
-                             f'following numeric values: {value}')
+        for quant in self.quantities:
+            if isinstance(value, quant.cls):
+                if isinstance(getattr(value, quant.value_prop), (int, float)):
+                    return super().encode_value(value)
+                else:
+                    raise ValueError(
+                        'Unit expressions are only allowed '
+                        f'following numeric values: {value}'
+                    )
 
         return super().encode_value(value)
 
@@ -745,8 +794,7 @@ class PDSLabelEncoder(ODLEncoder):
         self.convert_group_to_object = convert_group_to_object
         self.tab_replace = tab_replace
 
-    @staticmethod
-    def count_aggs(module: dict, obj_count: int = 0,
+    def count_aggs(self, module: abc.Mapping, obj_count: int = 0,
                    grp_count: int = 0) -> tuple((int, int)):
         """Returns the count of OBJECT and GROUP aggregations
         that are contained within the *module* as a two-tuple
@@ -757,20 +805,20 @@ class PDSLabelEncoder(ODLEncoder):
         # may contain aggregations.
 
         for k, v in module.items():
-            if isinstance(v, dict):
-                if isinstance(v, PVLGroup):
+            if isinstance(v, abc.Mapping):
+                if isinstance(v, self.grpcls):
                     grp_count += 1
-                elif isinstance(v, PVLObject):
+                elif isinstance(v, self.objcls):
                     obj_count += 1
                 else:
                     # We treat other dict-like Python objects as
-                    # PVLObjects for the purposes of this count,
+                    # PVL Objects for the purposes of this count,
                     # because that is how they will be encoded.
                     obj_count += 1
 
-        return (obj_count, grp_count)
+        return obj_count, grp_count
 
-    def encode(self, module: dict) -> str:
+    def encode(self, module: abc.MutableMapping) -> str:
         """Extends the parent function, by adding a restriction.
         For PDS, if there are any GROUP elements, there must be at
         least one OBJECT element in the label.  Behavior here
@@ -784,14 +832,14 @@ class PDSLabelEncoder(ODLEncoder):
                 for k, v in module.items():
                     # First try to convert any GROUPs that would not
                     # be valid PDS GROUPs.
-                    if isinstance(v, PVLGroup) and not self.is_PDSgroup(v):
-                        module[k] = PVLObject(v)
+                    if isinstance(v, self.grpcls) and not self.is_PDSgroup(v):
+                        module[k] = self.objcls(v)
                         break
                 else:
                     # Then just convert the first GROUP
                     for k, v in module.items():
-                        if isinstance(v, PVLGroup):
-                            module[k] = PVLObject(v)
+                        if isinstance(v, self.grpcls):
+                            module[k] = self.objcls(v)
                             break
                     else:
                         raise ValueError("Couldn't convert any of the GROUPs "
@@ -810,7 +858,7 @@ class PDSLabelEncoder(ODLEncoder):
         else:
             return s
 
-    def is_PDSgroup(self, group: dict) -> bool:
+    def is_PDSgroup(self, group: abc.Mapping) -> bool:
         """Returns true if the dict-like *group* qualifies as a PDS Group,
         false otherwise.
 
@@ -856,8 +904,13 @@ class PDSLabelEncoder(ODLEncoder):
             if k.startswith('^'):
                 if isinstance(v, int):
                     return False
-                elif isinstance(v, Units) and isinstance(v.value, int):
-                    return False
+                else:
+                    for quant in self.quantities:
+                        if(
+                            isinstance(v, quant.cls)
+                            and isinstance(getattr(v, quant.value_prop), int)
+                        ):
+                            return False
 
         # Item 2, no repeated keys:
         keys = list(group.keys())
@@ -879,9 +932,9 @@ class PDSLabelEncoder(ODLEncoder):
         # print('value at top:')
         # print(value)
 
-        if(isinstance(value, PVLGroup) and not self.is_PDSgroup(value)):
+        if isinstance(value, self.grpcls) and not self.is_PDSgroup(value):
             if self.convert_group_to_object:
-                value = PVLObject(value)
+                value = self.objcls(value)
             else:
                 raise ValueError('This GROUP element is not a valid PDS '
                                  'GROUP.  You could set '
diff --git a/pvl/parser.py b/pvl/parser.py
index d770a3f..6bd9ae7 100644
--- a/pvl/parser.py
+++ b/pvl/parser.py
@@ -56,7 +56,7 @@ will be thrown into the *tokens* generator iterator (via .throw()).
 import collections.abc as abc
 import re
 
-from ._collections import PVLModule, PVLGroup, PVLObject
+from .collections import MutableMappingSequence, PVLModule, PVLGroup, PVLObject
 from .token import Token
 from .grammar import PVLGrammar, OmniGrammar
 from .decoder import PVLDecoder, OmniDecoder
@@ -142,6 +142,7 @@ class PVLParser(object):
                  object_class=PVLObject):
 
         self.errors = []
+        self.doc = ""
 
         if lexer_fn is None:
             self.lexer = Lexer
@@ -165,23 +166,33 @@ class PVLParser(object):
         else:
             raise TypeError('The decode must be an instance of pvl.PVLDecoder.')
 
-        if issubclass(module_class, PVLModule):
+        if issubclass(module_class, MutableMappingSequence):
             self.modcls = module_class
         else:
-            raise TypeError('The module_class must be a subclass of PVLModule.')
+            raise TypeError(
+                'The module_class must be a '
+                'pvl.collections.MutableMappingSequence.'
+            )
 
-        if issubclass(group_class, PVLGroup):
+        if issubclass(group_class, MutableMappingSequence):
             self.grpcls = group_class
         else:
-            raise TypeError('The group_class must be a subclass of PVLGroup.')
+            raise TypeError(
+                'The group_class must be a '
+                'pvl.collections.MutableMappingSequence.'
+            )
 
-        if issubclass(object_class, PVLObject):
+        if issubclass(object_class, MutableMappingSequence):
             self.objcls = object_class
         else:
-            raise TypeError('The object_class must be a subclass of PVLObject.')
+            raise TypeError(
+                'The object_class must be a '
+                'pvl.collections.MutableMappingSequence.'
+            )
 
     def parse(self, s: str):
         """Converts the string, *s* to a PVLModule."""
+        self.doc = s
         tokens = self.lexer(s, g=self.grammar, d=self.decoder)
         module = self.parse_module(tokens)
         module.errors = sorted(self.errors)
@@ -255,7 +266,9 @@ class PVLParser(object):
                      'Statement, or an End Statement, but found '
                      f'"{t}" ')
 
-    def parse_module_post_hook(self, module, tokens: abc.Generator):
+    def parse_module_post_hook(
+            self, module: MutableMappingSequence, tokens: abc.Generator
+    ):
         """This function is meant to be overridden by subclasses
         that may want to perform some extra processing if
         'normal' parse_module() operations fail to complete.
@@ -313,7 +326,7 @@ class PVLParser(object):
                     self.parse_end_aggregation(begin, block_name, tokens)
                     break
 
-        return (block_name, agg)
+        return block_name, agg
 
     def parse_around_equals(self, tokens: abc.Generator) -> None:
         """Parses white space and comments on either side
@@ -375,7 +388,7 @@ class PVLParser(object):
 
         self.parse_statement_delimiter(tokens)
 
-        return(begin, str(block_name))
+        return begin, str(block_name)
 
     def parse_end_aggregation(self, begin_agg: str, block_name: str,
                               tokens: abc.Generator) -> None:
@@ -460,7 +473,6 @@ class PVLParser(object):
                                      <Value> [<Statement-Delimiter>]
 
         """
-        parameter_name = None
         try:
             t = next(tokens)
             if t.is_parameter_name():
@@ -473,12 +485,11 @@ class PVLParser(object):
             raise ValueError('Ran out of tokens before starting to parse '
                              'an Assignment-Statement.')
 
-        Value = None
         self.parse_around_equals(tokens)
 
         try:
             # print(f'parameter name: {parameter_name}')
-            Value = self.parse_value(tokens)
+            value = self.parse_value(tokens)
         except StopIteration:
             raise ParseError('Ran out of tokens to parse after the equals '
                              'sign in an Assignment-Statement: '
@@ -486,9 +497,10 @@ class PVLParser(object):
 
         self.parse_statement_delimiter(tokens)
 
-        return(parameter_name, Value)
+        return parameter_name, value
 
-    def parse_WSC_until(self, token: str, tokens: abc.Generator) -> bool:
+    @staticmethod
+    def parse_WSC_until(token: str, tokens: abc.Generator) -> bool:
         """Consumes objects from *tokens*, if the object's *.is_WSC()*
         function returns *True*, it will continue until *token* is
         encountered and will return *True*.  If it encounters an object
@@ -544,7 +556,7 @@ class PVLParser(object):
                              'While parsing, expected a comma (,)'
                              f'but found: "{t}"')
 
-    def parse_set(self, tokens: abc.Generator) -> set:
+    def parse_set(self, tokens: abc.Generator) -> frozenset:
         """Parses a PVL Set.
 
          <Set> ::= "{" <WSC>*
@@ -576,7 +588,8 @@ class PVLParser(object):
         """
         return self._parse_set_seq(self.grammar.sequence_delimiters, tokens)
 
-    def parse_statement_delimiter(self, tokens: abc.Generator) -> bool:
+    @staticmethod
+    def parse_statement_delimiter(tokens: abc.Generator) -> bool:
         """Parses the tokens for a Statement Delimiter.
 
         *tokens* is expected to be a *generator iterator* which
@@ -763,7 +776,9 @@ class OmniParser(PVLParser):
 
         return super().parse(nodash)
 
-    def parse_module_post_hook(self, module, tokens: abc.Generator):
+    def parse_module_post_hook(
+            self, module: MutableMappingSequence, tokens: abc.Generator
+    ):
         """Overrides the parent function to allow for more
         permissive parsing.  If an Assignment-Statement
         is blank, then the value will be assigned an
@@ -790,13 +805,12 @@ class OmniParser(PVLParser):
                     # Now use last_token as the parameter name
                     # for the next assignment, and we must
                     # reproduce the last part of parse-assignment:
-                    Value = None
                     try:
                         # print(f'parameter name: {last_token}')
                         self.parse_WSC_until(None, tokens)
-                        Value = self.parse_value(tokens)
+                        value = self.parse_value(tokens)
                         self.parse_statement_delimiter(tokens)
-                        module.append(str(last_token), Value)
+                        module.append(str(last_token), value)
                     except StopIteration:
                         module.append(str(last_token),
                                       self._empty_value(t.pos + 1))
@@ -814,10 +828,10 @@ class OmniParser(PVLParser):
             # see if we're at the end of tokens, which we want to handle.
             t = next(tokens)
             tokens.send(t)
-            return (module, True)  # keep parsing
+            return module, True  # keep parsing
         except StopIteration:
             # If we're out of tokens, that's okay.
-            return (module, False)  # return through parse_module()
+            return module, False  # return through parse_module()
 
     def parse_assignment_statement(self, tokens: abc.Generator) -> tuple:
         """Extends the parent function to allow for more
@@ -830,7 +844,7 @@ class OmniParser(PVLParser):
         except ParseError as err:
             if err.token is not None:
                 after_eq = self.doc.find('=', err.token.pos) + 1
-                return (str(err.token), self._empty_value(after_eq))
+                return str(err.token), self._empty_value(after_eq)
             else:
                 raise
 
diff --git a/setup.cfg b/setup.cfg
index 7caeffc..1145122 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -1,12 +1,5 @@
-[wheel]
-universal = 1
-
-[tool:pytest]
-doctest_optionflags = NORMALIZE_WHITESPACE
-
-
 [bumpversion]
-current_version = 1.0.0-alpha.7
+current_version = 1.0.0-alpha.8
 commit = False
 tag = False
 parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)(\-(?P<prerelease>[a-z]+)\.((?P<serial>\d+)))?
@@ -14,6 +7,12 @@ serialize =
 	{major}.{minor}.{patch}-{prerelease}.{serial}
 	{major}.{minor}.{patch}
 
+[wheel]
+universal = 1
+
+[tool:pytest]
+doctest_optionflags = NORMALIZE_WHITESPACE
+
 [bumpversion:part:prerelease]
 optional_value = beta
 values = 
diff --git a/setup.py b/setup.py
index c2b854a..9ec3945 100755
--- a/setup.py
+++ b/setup.py
@@ -12,7 +12,7 @@ history = open('HISTORY.rst').read().replace('.. :changelog:', '')
 
 setup(
     name='pvl',
-    version='1.0.0-alpha.7',
+    version='1.0.0-alpha.8',
     description='Python implementation of PVL (Parameter Value Language)',
     long_description=readme + '\n\n' + history,
     author='The PlanetaryPy Developers',


Test patch:
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 282e2ef..7366e20 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -342,15 +342,15 @@ def test_repr():
 def test_py3_items():
     module = pvl.PVLModule()
 
-    assert isinstance(module.items(), pvl._collections.ItemsView)
+    assert isinstance(module.items(), pvl.collections.ItemsView)
     with pytest.raises(IndexError):
         module.items()[0]
 
-    assert isinstance(module.keys(), pvl._collections.KeysView)
+    assert isinstance(module.keys(), pvl.collections.KeysView)
     with pytest.raises(IndexError):
         module.keys()[0]
 
-    assert isinstance(module.values(), pvl._collections.ValuesView)
+    assert isinstance(module.values(), pvl.collections.ValuesView)
     with pytest.raises(IndexError):
         module.values()[0]
 
@@ -360,7 +360,7 @@ def test_py3_items():
         ('a', 3),
     ])
 
-    assert isinstance(module.items(), pvl._collections.ItemsView)
+    assert isinstance(module.items(), pvl.collections.ItemsView)
     items = module.items()
     assert items[0] == ('a', 1)
     assert items[1] == ('b', 2)
@@ -369,7 +369,7 @@ def test_py3_items():
     assert items.index(('b', 2)) == 1
     assert items.index(('a', 3)) == 2
 
-    assert isinstance(module.keys(), pvl._collections.KeysView)
+    assert isinstance(module.keys(), pvl.collections.KeysView)
     keys = module.keys()
     assert keys[0] == 'a'
     assert keys[1] == 'b'
@@ -377,7 +377,7 @@ def test_py3_items():
     assert keys.index('a') == 0
     assert keys.index('b') == 1
 
-    assert isinstance(module.values(), pvl._collections.ValuesView)
+    assert isinstance(module.values(), pvl.collections.ValuesView)
     values = module.values()
     assert values[0] == 1
     assert values[1] == 2
@@ -402,19 +402,19 @@ def itervalues(module):
 def test_iterators():
     module = pvl.PVLModule()
 
-    assert isinstance(iteritems(module), pvl._collections.MappingView)
+    assert isinstance(iteritems(module), pvl.collections.MappingView)
     assert list(iteritems(module)) == []
     assert len(iteritems(module)) == 0
     assert isinstance(repr(iteritems(module)), str)
     assert ('a', 1) not in iteritems(module)
 
-    assert isinstance(iterkeys(module), pvl._collections.MappingView)
+    assert isinstance(iterkeys(module), pvl.collections.MappingView)
     assert list(iterkeys(module)) == []
     assert len(iterkeys(module)) == 0
     assert isinstance(repr(iterkeys(module)), str)
     assert 'a' not in iterkeys(module)
 
-    assert isinstance(itervalues(module), pvl._collections.MappingView)
+    assert isinstance(itervalues(module), pvl.collections.MappingView)
     assert list(itervalues(module)) == []
     assert len(itervalues(module)) == 0
     assert isinstance(repr(itervalues(module)), str)
@@ -426,7 +426,7 @@ def test_iterators():
         ('a', 3),
     ])
 
-    assert isinstance(iteritems(module), pvl._collections.MappingView)
+    assert isinstance(iteritems(module), pvl.collections.MappingView)
     assert list(iteritems(module)) == [('a', 1), ('b', 2), ('a', 3)]
     assert len(iteritems(module)) == 3
     assert isinstance(repr(iteritems(module)), str)
@@ -435,7 +435,7 @@ def test_iterators():
     assert ('a', 3) in iteritems(module)
     assert ('c', 4) not in iteritems(module)
 
-    assert isinstance(iterkeys(module), pvl._collections.MappingView)
+    assert isinstance(iterkeys(module), pvl.collections.MappingView)
     assert list(iterkeys(module)) == ['a', 'b', 'a']
     assert len(iterkeys(module)) == 3
     assert isinstance(repr(iterkeys(module)), str)
@@ -443,7 +443,7 @@ def test_iterators():
     assert 'b' in iterkeys(module)
     assert 'c' not in iterkeys(module)
 
-    assert isinstance(itervalues(module), pvl._collections.MappingView)
+    assert isinstance(itervalues(module), pvl.collections.MappingView)
     assert list(itervalues(module)) == [1, 2, 3]
     assert len(itervalues(module)) == 3
     assert isinstance(repr(itervalues(module)), str)
diff --git a/tests/test_decoder.py b/tests/test_decoder.py
index c9ac534..a26ba1d 100644
--- a/tests/test_decoder.py
+++ b/tests/test_decoder.py
@@ -20,7 +20,7 @@ import itertools
 import unittest
 
 from pvl.decoder import PVLDecoder, ODLDecoder, for_try_except
-from pvl._collections import Units
+from pvl.collections import Quantity
 
 
 class TestForTryExcept(unittest.TestCase):
@@ -127,7 +127,7 @@ class TestDecoder(unittest.TestCase):
 
     def test_decode_quantity(self):
         q = self.d.decode_quantity('15', 'm/s')
-        self.assertEqual(q, Units('15', 'm/s'))
+        self.assertEqual(q, Quantity('15', 'm/s'))
 
         try:
             from astropy import units as u
diff --git a/tests/test_encoder.py b/tests/test_encoder.py
index 9809938..0a0606e 100644
--- a/tests/test_encoder.py
+++ b/tests/test_encoder.py
@@ -4,7 +4,7 @@ import datetime
 import unittest
 
 from pvl.encoder import PVLEncoder, ODLEncoder, PDSLabelEncoder
-from pvl._collections import Units, PVLModule, PVLGroup, PVLObject
+from pvl.collections import Quantity, PVLModule, PVLGroup, PVLObject
 
 
 class TestDecoder(unittest.TestCase):
@@ -90,7 +90,7 @@ class TestDecoder(unittest.TestCase):
 
     def test_encode_value(self):
         pairs = ((42, '42'),
-                 (Units(34, 'm/s'), '34 <m/s>'))
+                 (Quantity(34, 'm/s'), '34 <m/s>'))
         for p in pairs:
             with self.subTest(pair=p):
                 self.assertEqual(p[1], self.e.encode_value(p[0]))
@@ -129,7 +129,7 @@ END;'''
         self.assertEqual(s, self.e.encode(m))
 
     def test_encode_quantity(self):
-        q, s = Units(34, 'm/s'), '34 <m/s>'
+        q, s = Quantity(34, 'm/s'), '34 <m/s>'
         self.assertEqual(s, self.e.encode_quantity(q))
 
         self.assertRaises(ValueError, self.e.encode_quantity, 'not a quant')
@@ -165,8 +165,8 @@ class TestODLDecoder(unittest.TestCase):
     def test_is_scalar(self):
         self.assertTrue(self.e.is_scalar(5))
         self.assertTrue(self.e.is_scalar('scalar'))
-        self.assertTrue(self.e.is_scalar(Units(5, 'm')))
-        self.assertFalse(self.e.is_scalar(Units('five', 'm')))
+        self.assertTrue(self.e.is_scalar(Quantity(5, 'm')))
+        self.assertFalse(self.e.is_scalar(Quantity('five', 'm')))
 
     def test_encode_quantity(self):
         try:
diff --git a/tests/test_parser.py b/tests/test_parser.py
index 22abbdf..d6685df 100644
--- a/tests/test_parser.py
+++ b/tests/test_parser.py
@@ -21,7 +21,7 @@ import unittest
 from pvl.parser import PVLParser, ParseError, OmniParser, EmptyValueAtLine
 from pvl.lexer import lexer as Lexer
 from pvl.lexer import LexerError
-from pvl._collections import Units, PVLModule, PVLGroup, PVLObject
+from pvl.collections import Quantity, PVLModule, PVLGroup, PVLObject
 
 
 class TestParse(unittest.TestCase):
@@ -119,10 +119,10 @@ class TestParse(unittest.TestCase):
         self.assertEqual(f, 'f')
 
     def test_parse_units(self):
-        pairs = ((Units(5, 'm'), '<m>'),
-                 (Units(5, 'm'), '< m >'),
-                 (Units(5, 'm /* comment */'), '< m /* comment */>'),
-                 (Units(5, 'm\nfoo'), '< m\nfoo >'))
+        pairs = ((Quantity(5, 'm'), '<m>'),
+                 (Quantity(5, 'm'), '< m >'),
+                 (Quantity(5, 'm /* comment */'), '< m /* comment */>'),
+                 (Quantity(5, 'm\nfoo'), '< m\nfoo >'))
         for p in pairs:
             with self.subTest(pairs=p):
                 tokens = Lexer(p[1])
@@ -166,7 +166,7 @@ class TestParse(unittest.TestCase):
                  ('Null', None),
                  ('TRUE', True),
                  ('false', False),
-                 ('9 <planets>', Units(9, 'planets')))
+                 ('9 <planets>', Quantity(9, 'planets')))
         for p in pairs:
             with self.subTest(pairs=p):
                 tokens = Lexer(p[0])
diff --git a/tests/test_pvl.py b/tests/test_pvl.py
index 3b94af1..977736b 100755
--- a/tests/test_pvl.py
+++ b/tests/test_pvl.py
@@ -11,7 +11,7 @@ import pvl
 from pvl import (PVLModule as Label,
                  PVLObject as LabelObject,
                  PVLGroup as LabelGroup)
-from pvl import Units
+from pvl import Quantity, Units
 
 
 DATA_DIR = os.path.join(os.path.dirname(__file__), 'data/')
@@ -717,20 +717,20 @@ def test_units():
         cool = (1 <number>)
         End
     """)
-    assert isinstance(label['foo'], Units)
+    assert isinstance(label['foo'], Quantity)
     assert label['foo'].value == 42
     assert label['foo'].units == 'beards'
 
-    assert isinstance(label['g'], Units)
+    assert isinstance(label['g'], Quantity)
     assert label['g'].value == 9.8
     assert label['g'].units == 'm/s'
 
-    assert isinstance(label['list'], Units)
+    assert isinstance(label['list'], Quantity)
     assert isinstance(label['list'].value, list)
     assert label['list'].units == 'numbers'
 
     assert isinstance(label['cool'], list)
-    assert isinstance(label['cool'][0], Units)
+    assert isinstance(label['cool'][0], Quantity)
     assert label['cool'][0].value == 1
     assert label['cool'][0].units == 'number'