Repository: voxel51/fiftyone
Issue: [BUG] Deleting dataset fields does not update DatasetDoc

In the example below, I `dataset.delete_frame_field("off_limits")` on my dataset, and then the field does not appear to be in my dataset's schema anymore:

```
Name:        short-clips-v4
Media type:  video
Num samples: 150
Persistent:  True
Tags:        []
Sample fields:
    id:       fiftyone.core.fields.ObjectIdField
    filepath: fiftyone.core.fields.StringField
    tags:     fiftyone.core.fields.ListField(fiftyone.core.fields.StringField)
    metadata: fiftyone.core.fields.EmbeddedDocumentField(fiftyone.core.metadata.Metadata)
Frame fields:
    id:           fiftyone.core.fields.ObjectIdField
    frame_number: fiftyone.core.fields.FrameNumberField
    detections:   fiftyone.core.fields.EmbeddedDocumentField(fiftyone.core.labels.Detections)
```

However, it still exists in `dataset._doc.frame_fields`, which causes confusion for the App, exporting, etc.

Changed files:
fiftyone/core/dataset.py, fiftyone/core/odm/mixins.py

diff --git a/fiftyone/core/dataset.py b/fiftyone/core/dataset.py
index 798e76d2b..5300f0f4c 100644
--- a/fiftyone/core/dataset.py
+++ b/fiftyone/core/dataset.py
@@ -321,8 +321,8 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
 
         if media_type not in fom.MEDIA_TYPES:
             raise ValueError(
-                'media_type can only be one of %s; received "%s"'
-                % (fom.MEDIA_TYPES, media_type)
+                "Invalid media_type '%s'. Supported values are %s"
+                % (media_type, fom.MEDIA_TYPES)
             )
 
         self._doc.media_type = media_type
@@ -350,6 +350,9 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
 
     @name.setter
     def name(self, name):
+        if name in list_datasets():
+            raise ValueError("A dataset with name '%s' already exists" % name)
+
         _name = self._doc.name
         try:
             self._doc.name = name
@@ -948,9 +951,7 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
         ) = _parse_field_mapping(field_mapping)
 
         if fields:
-            self._frame_doc_cls._rename_fields(
-                fields, new_fields, are_frame_fields=True
-            )
+            self._frame_doc_cls._rename_fields(fields, new_fields, frames=True)
             fofr.Frame._rename_fields(
                 self._frame_collection_name, fields, new_fields
             )
@@ -1243,7 +1244,7 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
 
         if fields:
             self._frame_doc_cls._delete_fields(
-                fields, are_frame_fields=True, error_level=error_level
+                fields, frames=True, error_level=error_level
             )
             fofr.Frame._purge_fields(self._frame_collection_name, fields)
 
@@ -2427,9 +2428,9 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
     ):
         """Adds the contents of the given archive to the dataset.
 
-        If the archive does not exist but a directory with the same root name
-        does exist, it is assumed that this directory contains the extracted
-        contents of the archive.
+        If a directory with the same root name as ``archive_path`` exists, it
+        is assumed that this directory contains the extracted contents of the
+        archive, and thus the archive is not re-extracted.
 
         See :ref:`this guide <loading-datasets-from-disk>` for example usages
         of this method and descriptions of the available dataset types.
@@ -2510,12 +2511,7 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
         Returns:
             a list of IDs of the samples that were added to the dataset
         """
-        dataset_dir = etau.split_archive(archive_path)[0]
-        if os.path.isfile(archive_path) or not os.path.isdir(dataset_dir):
-            etau.extract_archive(
-                archive_path, outdir=dataset_dir, delete_archive=cleanup
-            )
-
+        dataset_dir = _extract_archive_if_necessary(archive_path, cleanup)
         return self.add_dir(
             dataset_dir=dataset_dir,
             dataset_type=dataset_type,
@@ -2551,9 +2547,9 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
     ):
         """Merges the contents of the given archive into the dataset.
 
-        If the archive does not exist but a directory with the same root name
-        does exist, it is assumed that this directory contains the extracted
-        contents of the archive.
+        If a directory with the same root name as ``archive_path`` exists, it
+        is assumed that this directory contains the extracted contents of the
+        archive, and thus the archive is not re-extracted.
 
         See :ref:`this guide <loading-datasets-from-disk>` for example usages
         of this method and descriptions of the available dataset types.
@@ -2692,12 +2688,7 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
                 the :class:`fiftyone.utils.data.importers.DatasetImporter` for
                 the specified ``dataset_type``
         """
-        dataset_dir = etau.split_archive(archive_path)[0]
-        if os.path.isfile(archive_path) or not os.path.isdir(dataset_dir):
-            etau.extract_archive(
-                archive_path, outdir=dataset_dir, delete_archive=cleanup
-            )
-
+        dataset_dir = _extract_archive_if_necessary(archive_path, cleanup)
         return self.merge_dir(
             dataset_dir=dataset_dir,
             dataset_type=dataset_type,
@@ -3448,9 +3439,9 @@ class Dataset(foc.SampleCollection, metaclass=DatasetSingleton):
     ):
         """Creates a :class:`Dataset` from the contents of the given archive.
 
-        If the archive does not exist but a directory with the same root name
-        does exist, it is assumed that this directory contains the extracted
-        contents of the archive.
+        If a directory with the same root name as ``archive_path`` exists, it
+        is assumed that this directory contains the extracted contents of the
+        archive, and thus the archive is not re-extracted.
 
         See :ref:`this guide <loading-datasets-from-disk>` for example usages
         of this method and descriptions of the available dataset types.
@@ -5474,3 +5465,27 @@ def _parse_field_mapping(field_mapping):
             new_fields.append(new_field)
 
     return fields, new_fields, embedded_fields, embedded_new_fields
+
+
+def _extract_archive_if_necessary(archive_path, cleanup):
+    dataset_dir = etau.split_archive(archive_path)[0]
+
+    if not os.path.isdir(dataset_dir):
+        outdir = os.path.dirname(dataset_dir)
+        etau.extract_archive(
+            archive_path, outdir=outdir, delete_archive=cleanup
+        )
+
+        if not os.path.isdir(dataset_dir):
+            raise ValueError(
+                "Expected to find a directory '%s' after extracting '%s', "
+                "but it was not found" % (dataset_dir, archive_path)
+            )
+    else:
+        logger.info(
+            "Assuming '%s' contains the extracted contents of '%s'",
+            dataset_dir,
+            archive_path,
+        )
+
+    return dataset_dir
diff --git a/fiftyone/core/odm/mixins.py b/fiftyone/core/odm/mixins.py
index e7e55bde3..e748b8d90 100644
--- a/fiftyone/core/odm/mixins.py
+++ b/fiftyone/core/odm/mixins.py
@@ -390,15 +390,13 @@ class DatasetMixin(object):
         self.set_field(field_name, None)
 
     @classmethod
-    def _rename_fields(
-        cls, field_names, new_field_names, are_frame_fields=False
-    ):
+    def _rename_fields(cls, field_names, new_field_names, frames=False):
         """Renames the fields of the samples in this collection.
 
         Args:
             field_names: an iterable of field names
             new_field_names: an iterable of new field names
-            are_frame_fields (False): whether these are frame-level fields
+            frames (False): whether these are frame-level fields
         """
         default_fields = get_default_fields(
             cls.__bases__[0], include_private=True
@@ -417,9 +415,7 @@ class DatasetMixin(object):
             return
 
         for field_name, new_field_name in zip(field_names, new_field_names):
-            cls._rename_field_schema(
-                field_name, new_field_name, are_frame_fields
-            )
+            cls._rename_field_schema(field_name, new_field_name, frames)
 
         cls._rename_fields_simple(field_names, new_field_names)
 
@@ -528,14 +524,12 @@ class DatasetMixin(object):
         cls._clear_fields_collection(field_names, sample_collection)
 
     @classmethod
-    def _delete_fields(
-        cls, field_names, are_frame_fields=False, error_level=0
-    ):
+    def _delete_fields(cls, field_names, frames=False, error_level=0):
         """Deletes the field(s) from the samples in this collection.
 
         Args:
             field_names: an iterable of field names
-            are_frame_fields (False): whether these are frame-level fields
+            frames (False): whether these are frame-level fields
             error_level (0): the error level to use. Valid values are:
 
             -   0: raise error if a field cannot be deleted
@@ -568,7 +562,7 @@ class DatasetMixin(object):
             return
 
         for field_name in _field_names:
-            cls._delete_field_schema(field_name, are_frame_fields)
+            cls._delete_field_schema(field_name, frames)
 
         cls._delete_fields_simple(_field_names)
 
@@ -703,7 +697,6 @@ class DatasetMixin(object):
             if issubclass(cls, SampleDocument):
                 setattr(cls, field.name, field)
         except TypeError:
-            # Instance, not class
             pass
 
     @classmethod
@@ -735,14 +728,13 @@ class DatasetMixin(object):
         dataset_doc.save()
 
     @classmethod
-    def _rename_field_schema(
-        cls, field_name, new_field_name, are_frame_fields
-    ):
+    def _rename_field_schema(cls, field_name, new_field_name, frames):
         # pylint: disable=no-member
         field = cls._fields[field_name]
         field = _rename_field(field, new_field_name)
         cls._fields[new_field_name] = field
         del cls._fields[field_name]
+
         cls._fields_ordered = tuple(
             (fn if fn != field_name else new_field_name)
             for fn in cls._fields_ordered
@@ -753,19 +745,20 @@ class DatasetMixin(object):
             if issubclass(cls, Document):
                 setattr(cls, new_field_name, field)
         except TypeError:
-            # Instance, not class, so do not `setattr`
             pass
 
         dataset_doc = cls._dataset_doc()
 
-        if are_frame_fields:
+        if frames:
             for f in dataset_doc.frame_fields:
                 if f.name == field_name:
                     f.name = new_field_name
+                    f.db_field = new_field_name
         else:
             for f in dataset_doc.sample_fields:
                 if f.name == field_name:
                     f.name = new_field_name
+                    f.db_field = new_field_name
 
         dataset_doc.save()
 
@@ -776,7 +769,7 @@ class DatasetMixin(object):
         cls._add_field_schema(new_field_name, **get_field_kwargs(field))
 
     @classmethod
-    def _delete_field_schema(cls, field_name, are_frame_fields):
+    def _delete_field_schema(cls, field_name, frames):
         # pylint: disable=no-member
         del cls._fields[field_name]
         cls._fields_ordered = tuple(
@@ -786,7 +779,7 @@ class DatasetMixin(object):
 
         dataset_doc = cls._dataset_doc()
 
-        if are_frame_fields:
+        if frames:
             dataset_doc.frame_fields = [
                 f for f in dataset_doc.frame_fields if f.name != field_name
             ]