Skip to content
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

Deprecate (or remove?) legacy Matlab Toolbox in Cantera 3.1 #196

Closed
ischoegl opened this issue Feb 20, 2024 · 5 comments · Fixed by Cantera/cantera#1670
Closed

Deprecate (or remove?) legacy Matlab Toolbox in Cantera 3.1 #196

ischoegl opened this issue Feb 20, 2024 · 5 comments · Fixed by Cantera/cantera#1670
Labels
feature-request New feature request

Comments

@ischoegl
Copy link
Member

ischoegl commented Feb 20, 2024

Abstract

The "experimental" MATLAB toolbox is making progress (see #32, #177 and associated issues). At the same time, the "legacy" toolbox does not support many of the newer Cantera features (example: C++ Solution) and uses legacy implementations instead that prevent simplifications and thus creates added maintenance efforts.

Motivation

Describe the need for the proposed change:

  • What problem is it trying to solve? ... move MATLAB toolbox to modern code base
  • Who is affected by the change? ... Cantera community that uses the MATLAB interface
  • Why is this a good solution? ... the "legacy" toolbox is incomplete; the "experimental" toolbox is easier to maintain

Possible Solutions

All that is needed to do in for the legacy toolbox is to trigger deprecation warnings. The bulk of the work is in making the (currently experimental) re-implementation of the MATLAB toolbox ready for final release, i.e. #177. Tagging @rwest, @ssun30 for comments.

References

@ischoegl ischoegl added the feature-request New feature request label Feb 20, 2024
@rwest
Copy link
Member

rwest commented Feb 20, 2024

Thanks Ingmar. I guess #177 (Complete the new toolbox) is needed as well as this issue #196 (Deprecate the old toolbox).
The former seems to be a case of needing tests, deployment, and instructions, but maybe is best discussed on that thread instead.

@ischoegl
Copy link
Member Author

ischoegl commented Feb 20, 2024

Thanks Ingmar. I guess #177 (Complete the new toolbox) is needed as well as this issue #196 (Deprecate the old toolbox). The former seems to be a case of needing tests, deployment, and instructions, but maybe is best discussed on that thread instead.

Correct - changes to the new toolbox should be discussed in #177. At the same time, #177 has no impact on the deprecation cycle, which is why I opened this separate enhancement request. At least from my perspective, I am in favor of deprecating the old toolbox in the next release (Cantera 3.1), regardless of the state of #177.

@speth
Copy link
Member

speth commented Feb 20, 2024

I wonder if in this case there's a more efficient way of deprecating the old toolbox.

When we deprecate individual methods / classes within a given interface, the purpose of keeping the old methods in place with deprecation warnings is to give users a chance to update their code so it can work with both the current and next Cantera release. However, in the case of the new Matlab toolbox, that's not an option -- user code will have to be updated in ways that are not backwards compatible with the old toolbox.

As a result, I don't think there's much value in keeping the old Matlab toolbox in the 3.1 release and having it issue deprecation warnings every time you use it. Instead, I would suggest that we can update the downloads/installation sections of the website to note that the 3.0 release is the last version of the old toolbox, and that a future Cantera version (hopefully, but not necessarily 3.1) will include a stable version of the new toolbox. Anyone who needs the current toolbox can just stick to Cantera 3.0 until they're ready to upgrade. This approach would allow us remove the old toolbox from the repository immediately, rather than needing to wait for another Cantera release.

@ischoegl
Copy link
Member Author

@speth ... thanks for your comment. I guess I'd be 👍 for complete removal as well. I don't think that there will be any game-changers in 3.1 that would be usable from within the legacy MATLAB toolbox anyways.

@ischoegl ischoegl changed the title Deprecate legacy Matlab Toolbox in Cantera 3.1 Deprecate (or remove?) legacy Matlab Toolbox in Cantera 3.1 Feb 20, 2024
@ischoegl
Copy link
Member Author

Alright. Based on the reaction, I created a PR to remove the legacy MATLAB toolbox - see Cantera/cantera#1670.

In a nutshell: ~15k lines that are not under CI would be removed, which is pretty substantial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants