-
Notifications
You must be signed in to change notification settings - Fork 16
Make ModelingToolkit optional #209
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
Conversation
Thanks for doing this! The package still needs to use an ancient version of ModelingToolkit though, right? Or can it use Symbolics instead? |
Well, there is #204, but it was not touched in years and I do not know how much was missing. I would say this PR is still a direct step forward and Symbolics could be added later if anybody is willing to do the work - I think I am not :) EDIT: |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #209 +/- ##
==========================================
+ Coverage 74.86% 76.23% +1.37%
==========================================
Files 9 10 +1
Lines 541 547 +6
==========================================
+ Hits 405 417 +12
+ Misses 136 130 -6 ☔ View full report in Codecov by Sentry. |
Closing this in favor of #210. |
I hope CI runs through (I tested locally in v1.3 and v1.8) and - if so - somebody is willing to merge this and make a new (breaking) release 🙂
Summary
This PR makes
ModelingToolkit
an optional dependency, which allows to use this package in Julia v1.9 and newer again. The tests still useModelingToolkit
, which is why I downgraded the Julia version in CI. There are also some other minor fixes; in particular, I ran theAqua
tests and now it only still reports piracies and (external) ambiguities.Note that merging this PR will create clashes with #204, but that work seems to have been abandoned anyway.
Problem description with
ModelingToolkit
Only a very old version of
ModelingToolkit
is supported here (v3). There used to be an issue inAdapt
thatAdaptStaticArraysExt
cannot be compiled. This was fixed upstream, but the fix is inconsistent with the package bounds here. Hence compilation always fails in Julia v1.9 and newer.Short summary of commits (with the commit messages as names)
Added some files to
.gitignore
.Fixed some spacing issues in
NEWS.md
.Removed a redundant version requirement in
Project.toml
.Removed the
REQUIRE
files.Removed the
DS_Store
file.Use
isnothing
where applicable.Minor formatting of files.
Removed the seemingly redundant dependency
IntervalRootFinding
.Removed undefined exports. This closes #208.
This is the biggest change. It adds the dependency
Requires
(package extensions are not compatible, see description above) and wraps all code relevant toModelingToolkit
in functions that are loaded byRequires
.ModelingToolkit
is however still a dependency and is still loaded automatically.Actually removed
ModelingToolkit
as a dependency.Added a
test/Project.toml
file because the resolver of the package version would not work otherwise. The reason is that, to get the old v3 ofModelingToolkit
, the version ofStaticArrays
must be downgraded.Note that this will create clashes with #204, but that seems to have been abandoned anyway.
Removed a type parameter that was not used.
Added the coverage badge to the README. I was not sure whether it was omitted on purpose, but the CI script reports the coverage, which does not make sense to me if it is not shown anywhere.
Restricted the Julia versions used in CI to the lower and upper working versions.