Timer interface improvement
#3328
Conversation
|
If the optional handling is the path of choice, reconsider #3283 |
|
This means that the |
|
List of further optionals to change:
|
python/dolfinx/wrappers/common.cpp
Outdated
| @@ -152,8 +153,7 @@ void common(nb::module_& m) | |||
| nb::arg("global")); | |||
| // dolfinx::common::Timer | |||
| nb::class_<dolfinx::common::Timer>(m, "Timer", "Timer class") | |||
| .def(nb::init<>()) | |||
| .def(nb::init<std::string>(), nb::arg("task")) | |||
| .def(nb::init<std::string>(), nb::arg("task") = nb::none()) | |||
There was a problem hiding this comment.
Let's remove the default arg in the wrapper layer. Since we wrap the interface in the Python layer, best to handle the default arg there. Defaults in too many places can lead to bugs.
There was a problem hiding this comment.
This does not work, faced the same problem within #3322 - I'm not sure what causes this or how it may be fixed, but somehow only when the nb::none() is provided it properly picks up on the python None to cpp std::nullopt conversion.
There was a problem hiding this comment.
Yes, when it makes logical sense, i.e. when |
|
I'll file an issue for the remaining cases, then we can merge this one already. |
* Adapt Timer * Remove space * Switch to member variable being optional * clang-format * Fix python export * Remove move * Remove none from python export * Re add nb::none * Update python/dolfinx/wrappers/common.cpp --------- Co-authored-by: Garth N. Wells <gnw20@cam.ac.uk>
Timer interface simplified in both cpp and python part, making use of
std::optional.