-
Notifications
You must be signed in to change notification settings - Fork 6
Deprecate creation of empty Wall
or FlowDevice
objects in C++
#213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@speth … could you have a look at Cantera/cantera#1765, so I can move ahead and tackle this (as well as deprecate a bunch of raw pointered interfaces) prior to the release of 3.1? |
(repeating some comments from my review of Cantera/cantera#1788 here, since I think this is a better place for discussion) I like the idea of always providing the Currently, the ownership model is simple: none of the |
Thanks for the review of Cantera/cantera#1788 - before I address change requests, I wanted to briefly follow up on the Regarding the implementation under the hood, I think we need to increase the importance of |
For further thoughts on anticipated implementations see #201 (comment) |
@speth ... before revisiting Cantera/cantera#1788 (or, likely, a sequence of smaller PRs as there now is a rather large number of merge conflicts), I wanted to discuss one of the points you raise Cantera/cantera#1788 (review):
I think this is an interesting point, but I ultimately think that we need to better differentiate between implementation and visualization. I am partial to the idea of approaching this from the perspective of a 'bipartite graph' with Re-reading what I just wrote I realize that it's probably the terminology that is to blame: in the bipartite graph of Cantera/cantera#1788, the two types of objects are
I hope that this updated terminology will avoid erroneously conflating 'connectors' and 'edges'.
|
Abstract
Creation of empty
Wall
,FlowDevice
andReactorSurface
objects is not supported by the Python API, and Cantera/cantera#1765 removes preliminary support from experimental MATLAB. It would be consistent to disallow this at the C++ level as well, which would (eventually) simplify/streamline the interface by removing several methods and associated exception handling. Passing shared pointers toReactorBase
objects to constructors instead would be a significant step towards elimination of raw pointers from the reactor code base. A similar argument holds forReactorNet
and the pendingReactorEdge
(see Cantera/cantera#1697).Motivation
Describe the need for the proposed change:
Possible Solutions
Update constructors and factory methods and
clib
beyond changes introduced in Cantera/cantera#1765. Deprecate variousinstall
methods. Constructors should be updated to use signatures similar towhere null
upstream
/downstream
will raise deprecation warnings in 3.1, with defaults being removed thereafter.References
The text was updated successfully, but these errors were encountered: