Repository: idaholab/MontePy

Issue:
append_renumber() validates against old problem
**Describe the bug**

When copying numbered objects from one `MCNP_Problem` to another, one can use `NumberedObjectCollection.append_renumber(Numbered_MCNP_Object)` to avoid number conflicts--unless the _new_ number in the new problem is already in use in the old problem. This results in a fatal `NumberConflictError`.

Note that this only happens if a renumbering is necessary.

**To Reproduce**

``` python
prob1 = montepy.read_input("deck1.i")
prob2 = montepy.read_input("deck2.i")
prob2.append_renumber(prob1.materials[2])
```

**Error Message (if any)**

``` python
Traceback (most recent call last):
  File "./bugreport/montepy/numbered_object_collection.py", line 234, in append_renumber
    self.append(obj)
  File "./bugreport/montepy/numbered_object_collection.py", line 202, in append
    raise NumberConflictError(
montepy.errors.NumberConflictError: There was a numbering conflict when attempting to add MATERIAL: 2, ['iron'] to <class 'montepy.materials.Materials'>. Conflict was with MATERIAL: 2, ['iron']

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./bugreport/mwe.py", line 9, in <module>
    deck2.materials.append_renumber(m2)
  File "./montepy/numbered_object_collection.py", line 237, in append_renumber
    obj.number = number
  File "./bugreport/montepy/utilities.py", line 105, in setter
    validator(self, value)
  File "./bugreport/montepy/data_inputs/material.py", line 20, in _number_validator
    self._problem.materials.check_number(number)
  File "./bugreport/montepy/numbered_object_collection.py", line 98, in check_number
    raise NumberConflictError(
montepy.errors.NumberConflictError: Number 3 is already in use for the collection: <class 'montepy.materials.Materials'> by MATERIAL: 3, ['hydrogen', 'oxygen']

```

**MCNP input file snippet**

See first comment.

**Version**

 - Tested in versions 0.2.10 - 0.3.2


**Additional context**

Workaround: link the new numbered object to the new problem before calling `append_renumber()`.

`NumberedObjectCollection.append_renumber()` automatically links the appended numbered object to the new problem ([numbered_object_collection.py#L233-L241](https://github.com/idaholab/MontePy/blob/v0.3.2/montepy/numbered_object_collection.py#L233-L241)). It may be sufficient just to move this operation above the `try...except`.


Discussion:

Patch:
diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst
index 8f317767..c943806b 100644
--- a/doc/source/changelog.rst
+++ b/doc/source/changelog.rst
@@ -1,6 +1,14 @@
 MontePy Changelog
 =================
 
+#Next Version#
+--------------
+
+**Bug fixes**
+
+* Fixed bug with appending and renumbeing numbered objects from other MCNP problems (:issue:`466`).
+
+
 0.3.2
 --------------
 
diff --git a/montepy/numbered_object_collection.py b/montepy/numbered_object_collection.py
index 253caceb..9656b2d9 100644
--- a/montepy/numbered_object_collection.py
+++ b/montepy/numbered_object_collection.py
@@ -230,6 +230,8 @@ class NumberedObjectCollection(ABC):
         if not isinstance(step, int):
             raise TypeError("The step number must be an int")
         number = obj.number
+        if self._problem:
+            obj.link_to_problem(self._problem)
         try:
             self.append(obj)
         except NumberConflictError:
@@ -237,8 +239,6 @@ class NumberedObjectCollection(ABC):
             obj.number = number
             self.append(obj)
 
-        if self._problem:
-            obj.link_to_problem(self._problem)
         return number
 
     def request_number(self, start_num=1, step=1):


Test patch:
diff --git a/tests/test_numbered_collection.py b/tests/test_numbered_collection.py
index 623d6cc8..4b494ef7 100644
--- a/tests/test_numbered_collection.py
+++ b/tests/test_numbered_collection.py
@@ -115,6 +115,18 @@ class TestNumberedObjectCollection(unittest.TestCase):
         self.assertEqual(cell.number, 4)
         self.assertEqual(len(cells), size + 2)
 
+    def test_append_renumber_problems(self):
+        prob1 = copy.deepcopy(self.simple_problem)
+        prob2 = copy.deepcopy(self.simple_problem)
+        # Delete Material 2, making its number available.
+        prob2.materials.remove(prob2.materials[2])
+        len_mats = len(prob2.materials)
+        mat1 = prob1.materials[1]
+        new_num = prob2.materials.append_renumber(mat1)
+        assert new_num == 2, "Material not renumbered correctly."
+        assert len(prob2.materials) == len_mats + 1, "Material not appended"
+        assert prob2.materials[2] is mat1, "Material 2 is not the new material"
+
     def test_request_number(self):
         cells = self.simple_problem.cells
         self.assertEqual(cells.request_number(6), 6)