Fix inconsistent handling of figure names (backport #1430)

qiskit_experiments/framework/base_analysis.py

@@ -184,7 +184,7 @@ def run_analysis(expdata: ExperimentData):
                         if not result.experiment:
                             result.experiment = expdata.experiment_type
                         if not result.device_components:
-                            result.device_components = self._get_experiment_components(expdata)
+                            result.device_components = analysis._get_experiment_components(expdata)
                         if not result.backend:
                             result.backend = expdata.backend_name
                         if not result.created_time:
@@ -204,7 +204,7 @@ def run_analysis(expdata: ExperimentData):
                         if not result.experiment_id:
                             result.experiment_id = expdata.experiment_id
                         if not result.device_components:
-                            result.device_components = self._get_experiment_components(expdata)
+                            result.device_components = analysis._get_experiment_components(expdata)
                         if not result.experiment:
                             result.experiment = expdata.experiment_type
                         expdata.add_artifacts(result)
@@ -227,7 +227,7 @@ def run_analysis(expdata: ExperimentData):
                             name=f"{expdata.experiment_type}_{qubits_repr}_{short_id}.svg",
                         )
                     figure_to_add.append(figure)
-                expdata.add_figures(figure_to_add, figure_names=self.options.figure_names)
+                expdata.add_figures(figure_to_add, figure_names=analysis.options.figure_names)
 
         experiment_data.add_analysis_callback(run_analysis)
 

qiskit_experiments/framework/experiment_data.py

@@ -630,6 +630,7 @@ def _clear_results(self):
             self._deleted_figures.append(key)
         self._figures = ThreadSafeOrderedDict()
         self._artifacts = ThreadSafeOrderedDict()
+        self._db_data.figure_names.clear()
 
     @property
     def service(self) -> Optional[IBMExperimentService]:

releasenotes/notes/figure-names-inconsistency-afca1ac8e00fabac.yaml

@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    :class:`.ExperimentData` was updated so that running analysis a second time
+    with ``replace_results=True`` does not result in the ``figure_names``
+    property having incorrect data (both old and new figure names if the names
+    changed). See `#1430
+    <https://github.com/Qiskit-Extensions/qiskit-experiments/pull/1430>`__.
+  - |
+    :class:`.BaseAnalysis` was updated to respect ``figure_names`` as a keyword
+    argument to the ``run()`` method. Previously, this argument was ignored and
+    ``figure_names`` could only be set as an analysis option prior to calling
+    ``run()``. See `#1430
+    <https://github.com/Qiskit-Extensions/qiskit-experiments/pull/1430>`__.

test/database_service/test_db_experiment_data.py

@@ -1067,6 +1067,7 @@ def test_copy_metadata(self):
     def test_copy_figure_artifacts(self):
         """Test copy expdata figures and artifacts."""
         exp_data = FakeExperiment(experiment_type="qiskit_test").run(backend=FakeBackend())
+        self.assertExperimentDone(exp_data)
         exp_data.add_figures(str.encode("hello world"))
         exp_data.add_artifacts(ArtifactData(name="test", data="foo"))
         copied = exp_data.copy(copy_results=True)

test/framework/test_framework.py

@@ -28,6 +28,7 @@
 from qiskit_experiments.exceptions import AnalysisError
 from qiskit_experiments.framework import (
     ExperimentData,
+    FigureData,
     BaseExperiment,
     BaseAnalysis,
     AnalysisResultData,
@@ -163,6 +164,30 @@ def test_analysis_replace_results_true(self):
         self.assertEqualExtended(expdata1.analysis_results(), expdata2.analysis_results())
         self.assertEqual(result_ids, list(expdata2._deleted_analysis_results))
 
+    def test_analysis_replace_results_true_new_figure(self):
+        """Test running analysis with replace_results=True keeps figure data consistent"""
+        analysis = FakeAnalysis()
+        analysis.options.add_figures = True
+        analysis.options.figure_names = ["old_figure_name.svg"]
+
+        expdata = ExperimentData()
+        expdata.add_data(self.fake_job_data())
+        analysis.run(expdata, seed=54321)
+        self.assertExperimentDone(expdata)
+
+        # Assure all figure names map to valid figures
+        self.assertEqual(expdata.figure_names, ["old_figure_name.svg"])
+        self.assertIsInstance(expdata.figure("old_figure_name"), FigureData)
+
+        analysis.run(
+            expdata, replace_results=True, seed=12345, figure_names=["new_figure_name.svg"]
+        )
+        self.assertExperimentDone(expdata)
+
+        # Assure figure names have changed but are still valid
+        self.assertEqual(expdata.figure_names, ["new_figure_name.svg"])
+        self.assertIsInstance(expdata.figure("new_figure_name"), FigureData)
+
     def test_analysis_replace_results_false(self):
         """Test running analysis with replace_results=False"""
         analysis = FakeAnalysis()