Suggestions to PR 1589

Migration.md

@@ -18,17 +18,21 @@ but with improved human-readability..
    package.xml package name. Use `find_package(sdformat)` instead of
    `find_package(sdformatX)` going forward.
 
+### Additions
+
 - **sdf/Element.hh**:
+  + `sdf::ElementConstPtr ElementDescription(unsigned int) const`
+  + `sdf::ElementConstPtr ElementDescription(const std::string &) const`
   + `sdf::ElementPtr MutableElementDescription(unsigned int)`
   + `sdf::ElementPtr MutableElementDescription(const std::string &)`
 
 ### Deprecations
 - **sdf/Element.hh**:
   + Mutable access to an element description via `Element::GetElementDescription` has been deprecated. Please use `Element::MutableElementDescription` instead. For immutable access, please use `Element::ElementDescription`.
   + ***Deprecation:*** `sdf::ElementPtr GetElementDescription(unsigned int)`
-  + ***Replacement:*** `sdf::ElementPtr ElementDescription(unsigned int) const`
+  + ***Replacement:*** `sdf::ElementConstPtr ElementDescription(unsigned int) const`
   + ***Deprecation:*** `sdf::ElementPtr GetElementDescription(const std::string &)`
-  + ***Replacement:*** `sdf::ElementPtr ElementDescription(const std::string &) const`
+  + ***Replacement:*** `sdf::ElementConstPtr ElementDescription(const std::string &) const`
 
 ### Removals
 

python/src/sdf/pyElement.cc

@@ -61,7 +61,6 @@ void defineElement(py::object module)
   using PyClassElement = py::class_<Element, ElementPtr>;
 
   // TODO(azeey): GetElementDescription has been deprecated. Remove in sdformat17
-  GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
   auto warnings = py::module::import("warnings");
   auto builtins = py::module::import("builtins");
 
@@ -157,7 +156,9 @@ void defineElement(py::object module)
                   "GetElementDescription is deprecated. Use ElementDescription "
                   "or MutableElementDescription instead",
                    builtins.attr("DeprecationWarning"));
+            GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
             return _self.GetElementDescription(_index);
+            GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
           },
           "Get an element description using an index")
       .def(
@@ -169,7 +170,9 @@ void defineElement(py::object module)
               "or MutableElementDescription instead",
               builtins.attr("DeprecationWarning"));
 
+            GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
             return _self.GetElementDescription(_key);
+            GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
           },
           "Get an element description using a key")
       .def("element_description",
@@ -261,7 +264,6 @@ void defineElement(py::object module)
            "Set a text description for the element.")
       .def("add_element_description", &Element::AddElementDescription,
            "Add a new element description");
-  GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
 
   // Definitions for `Get<T>`, and `Set<T>` which will bind to `get_bool`, 
   // `get_int`, etc.

python/test/pyElement_TEST.py

@@ -485,7 +485,6 @@ def test_mutable_element_description(self):
 
         elem_clone = elem.clone()
         self.assertEqual(elem_clone.element_description("radius"), desc)
-        self.assertEqual(elem_clone.element_description("radius"), desc)
         self.assertEqual(elem_clone.element_description(0), desc)
 
         with warnings.catch_warnings():

src/Element.cc

@@ -690,6 +690,7 @@ void ElementPrivate::PrintAttributes(sdf::Errors &_errors,
   }
 }
 
+/////////////////////////////////////////////////
 std::optional<unsigned int> ElementPrivate::ElementDescriptionIndex(
     const std::string &_key)
 {
@@ -908,7 +909,7 @@ size_t Element::GetElementDescriptionCount() const
   return this->dataPtr->elementDescriptions.size();
 }
 
-///////////////////////////////////////////////
+/////////////////////////////////////////////////
 ElementPtr Element::GetElementDescription(unsigned int _index) const
 {
   return std::const_pointer_cast<Element>(this->ElementDescription(_index));
@@ -956,11 +957,11 @@ ElementPtr Element::MutableElementDescription(unsigned int _index)
     return ElementPtr();
   }
 
-  // To improve performance of the sdformat library, element descriptions are
-  // shared amont elements of the same type. If the user requests a mutable
+  // To improve the performance of libsdformat, element descriptions are
+  // shared among elements of the same type. If the user requests a mutable
   // element description, we make a clone here so that other elements are not
   // affected by the potential modifications the user makes. We keep track of
-  // the clones we've made so that it a clone is made at most once for each
+  // the clones we've made so that a clone is made at most once for each
   // element description.
   if (this->dataPtr->clonedElementDescriptions.count(desc) == 0)
   {