Repository: idaholab/MontePy

Issue:
Deprecation in support of material redesign
Assuming #504  is true and a major release is needed, deprecation warnings are needed. I think `Material.material_components` should raise a `DeprecationWarning` pointing to #504 with release 0.4.1.

Also I think with the next major release a lot of the DeprecationWarnings from Montepy 0.2.0 can be removed now. Just as: `montepy.data_cards`. I think at least the ones that are raised as an Error because changing a `DeprecationWarning` for an `AttributeError` won't be too bad. 

I'm not sure if these new Deprecations should occur before or after a migration plan is in place. The migration plan would need to have the new interface pretty well pinned down, so I'm hesitant to wait. Maybe we could at least make a landing page in the documentation that we can explain the issues, and possible migration, alpha testing etc. 

Discussion:

Patch:
diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst
index 89e89561..ce5bdbe5 100644
--- a/doc/source/changelog.rst
+++ b/doc/source/changelog.rst
@@ -10,7 +10,11 @@ MontePy Changelog
 
 **Performance Improvement**
 
-* Fixed method of linking ``Material`` to ``ThermalScattering`` objects, avoiding a very expensive O(N:sup:`2`) (:issue:`510`).
+* Fixed method of linking ``Material`` to ``ThermalScattering`` objects, avoiding a very expensive O(N :sup:`2`) (:issue:`510`).
+
+**Deprecations**
+
+* Marked ``Material.material_components`` as deprecated, and created migration plan describing what to expect moving forward (:issue:`506`).
 
 0.4.0
 --------------
diff --git a/doc/source/migrations/migrate0_1.rst b/doc/source/migrations/migrate0_1.rst
new file mode 100644
index 00000000..945fd2d6
--- /dev/null
+++ b/doc/source/migrations/migrate0_1.rst
@@ -0,0 +1,54 @@
+.. _migrate 0 1:
+
+Migration plan for MontePy 1.0.0
+================================
+
+.. meta::
+   :description: Migration plan for moving from MontePy 0.x to MontePy 1.0.0
+
+Necessity
+---------
+
+The MCNP 6.2 and 6.3 manuals are ambiguously worded around whether nuclides can be repeated in a material definition.
+Due to this the authors of MontePy in MontePy 0.x.x assumed that repeated nuclides were not allowed.
+Due to this assumption material composition data were stored in  python dictionary,
+where the keys were the nuclide and their library.
+Due to this if duplicate nuclides are present only the last instance of that nuclide will have its information preserved in the model.
+This is clearly not a desired outcome.
+
+However, it has been confirmed that  duplicate nuclides are allowed in MCNP,
+and can be advantageous. 
+See :issue:`504` for more details.
+Due to this it was decided that the best way forward was to abandon the old design,
+and to create a brand new data structure.
+This means that backwards compatibility *will* be broken, 
+and so this fix is leading to a major version release.
+
+
+Deprecations
+------------
+The following properties and objects are currently deprecated, 
+and will be removed in MontePy 1.0.0.
+
+* :func:`montepy.data_inputs.material.Material.material_components`. 
+  This is the dictionary that caused this design problem. 
+
+* :class:`montepy.data_inputs.material_components.MaterialComponents`
+  This is the class that stores information in the above dictionary. 
+  It is largely excess object wrapping, that makes the material interface 
+  overly complex.
+
+* :class:`montepy.data_inputs.Isotope` will be renamed to ``Nuclide``. 
+  This is to better align with MCNP documentation,
+  and better reflect that the nuclear data for a nuclide can represent 
+  isotopic, isomeric, or atomic data.
+
+
+New Interface
+-------------
+Currently the replacement interface has not been fully designed yet.
+If you have input you can `join the discussion <https://github.com/idaholab/MontePy/discussions/475>`_.
+There will also be some alpha-testing announced in that discussion.
+
+Once MontePy 1.0.0 is released this will be updated with information about the new interface,
+and how to migrate to it.
diff --git a/doc/source/users.rst b/doc/source/users.rst
index 7a572473..0e18411d 100644
--- a/doc/source/users.rst
+++ b/doc/source/users.rst
@@ -9,5 +9,6 @@ User Guide
    utilities
    tricks
    faq
+   migrations/migrate0_1
    changelog
    publications
diff --git a/montepy/data_inputs/isotope.py b/montepy/data_inputs/isotope.py
index 81a3d0a3..a0f5a78c 100644
--- a/montepy/data_inputs/isotope.py
+++ b/montepy/data_inputs/isotope.py
@@ -3,13 +3,21 @@ from montepy.data_inputs.element import Element
 from montepy.errors import *
 from montepy.input_parser.syntax_node import PaddingNode, ValueNode
 
+import warnings
+
 
 class Isotope:
     """
     A class to represent an MCNP isotope
 
+    .. deprecated:: 0.4.1
+        This will class is deprecated, and will be renamed: ``Nuclde``.
+        For more details see the :ref:`migrate 0 1`.
+
     :param ZAID: the MCNP isotope identifier
     :type ZAID: str
+    :param suppress_warning: Whether to suppress the ``FutureWarning``.
+    :type suppress_warning: bool
     """
 
     #                   Cl-52      Br-101     Xe-150      Os-203    Cm-251     Og-296
@@ -22,7 +30,14 @@ class Isotope:
     Points on bounding curve for determining if "valid" isotope
     """
 
-    def __init__(self, ZAID="", node=None):
+    def __init__(self, ZAID="", node=None, suppress_warning=False):
+        if not suppress_warning:
+            warnings.warn(
+                "montepy.data_inputs.isotope.Isotope is deprecated and will be renamed: Nuclide.\n"
+                "See <https://www.montepy.org/migrations/migrate0_1.html> for more information ",
+                FutureWarning,
+            )
+
         if node is not None and isinstance(node, ValueNode):
             if node.type == float:
                 node = ValueNode(node.token, str, node.padding)
diff --git a/montepy/data_inputs/material.py b/montepy/data_inputs/material.py
index 0c754e95..fab0dbfa 100644
--- a/montepy/data_inputs/material.py
+++ b/montepy/data_inputs/material.py
@@ -12,6 +12,8 @@ from montepy.utilities import *
 import itertools
 import re
 
+import warnings
+
 
 def _number_validator(self, number):
     if number <= 0:
@@ -51,7 +53,7 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
                     f"Material definitions for material: {self.number} is not valid.",
                 )
             for isotope_node, fraction in iterator:
-                isotope = Isotope(node=isotope_node)
+                isotope = Isotope(node=isotope_node, suppress_warning=True)
                 fraction.is_negatable_float = True
                 if not set_atom_frac:
                     set_atom_frac = True
@@ -68,8 +70,9 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
                             input,
                             f"Material definitions for material: {self.number} cannot use atom and mass fraction at the same time",
                         )
+
                 self._material_components[isotope] = MaterialComponent(
-                    isotope, fraction
+                    isotope, fraction, suppress_warning=True
                 )
 
     @make_prop_val_node("_old_number")
@@ -102,13 +105,23 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
     @property
     def material_components(self):
         """
-        The internal dictionary containing all the components of this material.
+            The internal dictionary containing all the components of this material.
 
-        The keys are :class:`~montepy.data_inputs.isotope.Isotope` instances, and the values are
-        :class:`~montepy.data_inputs.material_component.MaterialComponent` instances.
+        .. deprecated:: 0.4.1
+            MaterialComponent has been deprecated as part of a redesign for the material
+            interface due to a critical bug in how MontePy handles duplicate nuclides.
+            See :ref:`migrate 0 1`.
 
-        :rtype: dict
+            The keys are :class:`~montepy.data_inputs.isotope.Isotope` instances, and the values are
+            :class:`~montepy.data_inputs.material_component.MaterialComponent` instances.
+
+            :rtype: dict
         """
+        warnings.warn(
+            f"""material_components is deprecated, and will be removed in MontePy 1.0.0.
+See <https://www.montepy.org/migrations/migrate0_1.html> for more information """,
+            DeprecationWarning,
+        )
         return self._material_components
 
     @make_prop_pointer("_thermal_scattering", thermal_scattering.ThermalScatteringLaw)
@@ -149,7 +162,7 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
 
     def _update_values(self):
         new_list = syntax_node.IsotopesNode("new isotope list")
-        for isotope, component in self.material_components.items():
+        for isotope, component in self._material_components.items():
             isotope._tree.value = isotope.mcnp_str()
             new_list.append(("_", isotope._tree, component._tree))
         self._tree.nodes["data"] = new_list
@@ -198,8 +211,8 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
         else:
             ret += "mass\n"
 
-        for component in self.material_components:
-            ret += repr(self.material_components[component]) + "\n"
+        for component in self._material_components:
+            ret += repr(self._material_components[component]) + "\n"
         if self.thermal_scattering:
             ret += f"Thermal Scattering: {self.thermal_scattering}"
 
@@ -212,7 +225,7 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
     def _get_material_elements(self):
         sortable_components = [
             (iso, component.fraction)
-            for iso, component in self.material_components.items()
+            for iso, component in self._material_components.items()
         ]
         sorted_comps = sorted(sortable_components)
         elements_set = set()
@@ -224,7 +237,7 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
         return elements
 
     def validate(self):
-        if len(self.material_components) == 0:
+        if len(self._material_components) == 0:
             raise IllegalState(
                 f"Material: {self.number} does not have any components defined."
             )
@@ -237,10 +250,10 @@ class Material(data_input.DataInputAbstract, Numbered_MCNP_Object):
 
         """
         temp_hash = ""
-        sorted_isotopes = sorted(list(self.material_components.keys()))
+        sorted_isotopes = sorted(list(self._material_components.keys()))
         for isotope in sorted_isotopes:
             temp_hash = hash(
-                (temp_hash, str(isotope), self.material_components[isotope].fraction)
+                (temp_hash, str(isotope), self._material_components[isotope].fraction)
             )
 
         return hash((temp_hash, self.number))
diff --git a/montepy/data_inputs/material_component.py b/montepy/data_inputs/material_component.py
index fcb4bd49..e8738575 100644
--- a/montepy/data_inputs/material_component.py
+++ b/montepy/data_inputs/material_component.py
@@ -3,6 +3,8 @@ from montepy.data_inputs.isotope import Isotope
 from montepy.input_parser.syntax_node import PaddingNode, ValueNode
 from montepy.utilities import make_prop_val_node
 
+import warnings
+
 
 def _enforce_positive(self, val):
     if val <= 0:
@@ -15,13 +17,26 @@ class MaterialComponent:
 
     For example: this may be H-1 in water: like 1001.80c — 0.6667
 
+    .. deprecated:: 0.4.1
+        MaterialComponent has been deprecated as part of a redesign for the material
+        interface due to a critical bug in how MontePy handles duplicate nuclides.
+        See :ref:`migrate 0 1`.
+
     :param isotope: the Isotope object representing this isotope
     :type isotope: Isotope
     :param fraction: the fraction of this component in the material
     :type fraction: ValueNode
+    :param suppress_warning: Whether to suppress the ``DeprecationWarning``.
+    :type suppress_warning: bool
     """
 
-    def __init__(self, isotope, fraction):
+    def __init__(self, isotope, fraction, suppress_warning=False):
+        if not suppress_warning:
+            warnings.warn(
+                f"""MaterialComponent is deprecated, and will be removed in MontePy 1.0.0.
+See <https://www.montepy.org/migrations/migrate0_1.html> for more information """,
+                DeprecationWarning,
+            )
         if not isinstance(isotope, Isotope):
             raise TypeError(f"Isotope must be an Isotope. {isotope} given")
         if isinstance(fraction, (float, int)):


Test patch:
diff --git a/tests/test_material.py b/tests/test_material.py
index 603b92c7..b4a6de76 100644
--- a/tests/test_material.py
+++ b/tests/test_material.py
@@ -87,11 +87,16 @@ def test_material_format_mcnp():
 )
 def test_material_comp_init(isotope, conc, error):
     with pytest.raises(error):
-        MaterialComponent(Isotope(isotope), conc)
+        MaterialComponent(Isotope(isotope, suppress_warning=True), conc, True)
+
+
+def test_mat_comp_init_warn():
+    with pytest.warns(DeprecationWarning):
+        MaterialComponent(Isotope("1001.80c", suppress_warning=True), 0.1)
 
 
 def test_material_comp_fraction_setter():
-    comp = MaterialComponent(Isotope("1001.80c"), 0.1)
+    comp = MaterialComponent(Isotope("1001.80c", suppress_warning=True), 0.1, True)
     comp.fraction = 5.0
     assert comp.fraction == pytest.approx(5.0)
     with pytest.raises(ValueError):
@@ -101,7 +106,7 @@ def test_material_comp_fraction_setter():
 
 
 def test_material_comp_fraction_str():
-    comp = MaterialComponent(Isotope("1001.80c"), 0.1)
+    comp = MaterialComponent(Isotope("1001.80c", suppress_warning=True), 0.1, True)
     str(comp)
     repr(comp)
 
@@ -115,23 +120,24 @@ def test_material_update_format():
     print(material.format_for_mcnp_input((6, 2, 0)))
     assert "8016" in material.format_for_mcnp_input((6, 2, 0))[0]
     # addition
-    isotope = Isotope("2004.80c")
-    material.material_components[isotope] = MaterialComponent(isotope, 0.1)
-    print(material.format_for_mcnp_input((6, 2, 0)))
-    assert "2004" in material.format_for_mcnp_input((6, 2, 0))[0]
-    # update
-    isotope = list(material.material_components.keys())[-1]
-    print(material.material_components.keys())
-    material.material_components[isotope].fraction = 0.7
-    print(material.format_for_mcnp_input((6, 2, 0)))
-    assert "0.7" in material.format_for_mcnp_input((6, 2, 0))[0]
-    material.material_components[isotope] = MaterialComponent(isotope, 0.6)
-    print(material.format_for_mcnp_input((6, 2, 0)))
-    assert "0.6" in material.format_for_mcnp_input((6, 2, 0))[0]
-    # delete
-    del material.material_components[isotope]
-    print(material.format_for_mcnp_input((6, 2, 0)))
-    assert "8016" in material.format_for_mcnp_input((6, 2, 0))[0]
+    isotope = Isotope("2004.80c", suppress_warning=True)
+    with pytest.deprecated_call():
+        material.material_components[isotope] = MaterialComponent(isotope, 0.1, True)
+        print(material.format_for_mcnp_input((6, 2, 0)))
+        assert "2004" in material.format_for_mcnp_input((6, 2, 0))[0]
+        # update
+        isotope = list(material.material_components.keys())[-1]
+        print(material.material_components.keys())
+        material.material_components[isotope].fraction = 0.7
+        print(material.format_for_mcnp_input((6, 2, 0)))
+        assert "0.7" in material.format_for_mcnp_input((6, 2, 0))[0]
+        material.material_components[isotope] = MaterialComponent(isotope, 0.6, True)
+        print(material.format_for_mcnp_input((6, 2, 0)))
+        assert "0.6" in material.format_for_mcnp_input((6, 2, 0))[0]
+        # delete
+        del material.material_components[isotope]
+        print(material.format_for_mcnp_input((6, 2, 0)))
+        assert "8016" in material.format_for_mcnp_input((6, 2, 0))[0]
 
 
 @pytest.mark.parametrize(
@@ -157,8 +163,9 @@ def test_material_init(line, mat_number, is_atom, fractions):
     assert material.number == mat_number
     assert material.old_number == mat_number
     assert material.is_atom_fraction == is_atom
-    for component, gold in zip(material.material_components.values(), fractions):
-        assert component.fraction == pytest.approx(gold)
+    with pytest.deprecated_call():
+        for component, gold in zip(material.material_components.values(), fractions):
+            assert component.fraction == pytest.approx(gold)
     if "gas" in line:
         assert material.parameters["gas"]["data"][0].value == pytest.approx(1.0)
 
@@ -175,28 +182,29 @@ def test_bad_init(line):
 
 class TestIsotope(TestCase):
     def test_isotope_init(self):
-        isotope = Isotope("1001.80c")
+        with pytest.warns(FutureWarning):
+            isotope = Isotope("1001.80c")
         self.assertEqual(isotope.ZAID, "1001")
         self.assertEqual(isotope.Z, 1)
         self.assertEqual(isotope.A, 1)
         self.assertEqual(isotope.element.Z, 1)
         self.assertEqual(isotope.library, "80c")
         with self.assertRaises(ValueError):
-            Isotope("1001.80c.5")
+            Isotope("1001.80c.5", suppress_warning=True)
         with self.assertRaises(ValueError):
-            Isotope("hi.80c")
+            Isotope("hi.80c", suppress_warning=True)
 
     def test_isotope_metastable_init(self):
-        isotope = Isotope("13426.02c")
+        isotope = Isotope("13426.02c", suppress_warning=True)
         self.assertEqual(isotope.ZAID, "13426")
         self.assertEqual(isotope.Z, 13)
         self.assertEqual(isotope.A, 26)
         self.assertTrue(isotope.is_metastable)
         self.assertEqual(isotope.meta_state, 1)
-        isotope = Isotope("92635.02c")
+        isotope = Isotope("92635.02c", suppress_warning=True)
         self.assertEqual(isotope.A, 235)
         self.assertEqual(isotope.meta_state, 1)
-        isotope = Isotope("92935.02c")
+        isotope = Isotope("92935.02c", suppress_warning=True)
         self.assertEqual(isotope.A, 235)
         self.assertEqual(isotope.meta_state, 4)
         self.assertEqual(isotope.mcnp_str(), "92935.02c")
@@ -208,45 +216,45 @@ class TestIsotope(TestCase):
             ("77764", 77, 164, 3),
         ]
         for ZA, Z_ans, A_ans, isomer_ans in edge_cases:
-            isotope = Isotope(ZA + ".80c")
+            isotope = Isotope(ZA + ".80c", suppress_warning=True)
             self.assertEqual(isotope.Z, Z_ans)
             self.assertEqual(isotope.A, A_ans)
             self.assertEqual(isotope.meta_state, isomer_ans)
         with self.assertRaises(ValueError):
-            isotope = Isotope("13826.02c")
+            isotope = Isotope("13826.02c", suppress_warning=True)
 
     def test_isotope_get_base_zaid(self):
-        isotope = Isotope("92635.02c")
+        isotope = Isotope("92635.02c", suppress_warning=True)
         self.assertEqual(isotope.get_base_zaid(), 92235)
 
     def test_isotope_library_setter(self):
-        isotope = Isotope("1001.80c")
+        isotope = Isotope("1001.80c", suppress_warning=True)
         isotope.library = "70c"
         self.assertEqual(isotope.library, "70c")
         with self.assertRaises(TypeError):
             isotope.library = 1
 
     def test_isotope_str(self):
-        isotope = Isotope("1001.80c")
+        isotope = Isotope("1001.80c", suppress_warning=True)
         assert isotope.mcnp_str() == "1001.80c"
         assert isotope.nuclide_str() == "H-1.80c"
         assert repr(isotope) == "Isotope('H-1.80c')"
         assert str(isotope) == " H-1     (80c)"
-        isotope = Isotope("94239.80c")
+        isotope = Isotope("94239.80c", suppress_warning=True)
         assert isotope.nuclide_str() == "Pu-239.80c"
         assert isotope.mcnp_str() == "94239.80c"
         assert repr(isotope) == "Isotope('Pu-239.80c')"
-        isotope = Isotope("92635.80c")
+        isotope = Isotope("92635.80c", suppress_warning=True)
         assert isotope.nuclide_str() == "U-235m1.80c"
         assert isotope.mcnp_str() == "92635.80c"
         assert str(isotope) == " U-235m1 (80c)"
         assert repr(isotope) == "Isotope('U-235m1.80c')"
         # stupid legacy stupidity #486
-        isotope = Isotope("95642")
+        isotope = Isotope("95642", suppress_warning=True)
         assert isotope.nuclide_str() == "Am-242"
         assert isotope.mcnp_str() == "95642"
         assert repr(isotope) == "Isotope('Am-242')"
-        isotope = Isotope("95242")
+        isotope = Isotope("95242", suppress_warning=True)
         assert isotope.nuclide_str() == "Am-242m1"
         assert isotope.mcnp_str() == "95242"
         assert repr(isotope) == "Isotope('Am-242m1')"