-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
Fix compiler warnings #1519
Fix compiler warnings #1519
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1519 +/- ##
==========================================
- Coverage 70.51% 70.50% -0.02%
==========================================
Files 376 376
Lines 59072 59075 +3
Branches 21217 21219 +2
==========================================
- Hits 41653 41649 -4
- Misses 14344 14348 +4
- Partials 3075 3078 +3
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
e3221ed
to
e55848f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 ... thanks, @speth. This all looks good to me!
Fixes a confusing error (depending on the LAPACK implementation) if the reactor is initialized before setting the mass flow rate.
For warnings from pure Python methods, we need the 'stacklevel=2' argument to show the call in the users' code. For Cythonized methods, the default 'stacklevel=1' gives the desired result because the Cython method is not visible as part of the Python stacktrace.
Changes proposed in this pull request
yaml-cpp
being marked as DLL export that MSVC thinks maybe shouldn't be.FlowReactor
. On macOS usingopenblas
, not doing this before callinginitialize
would lead to a confusing LAPACK error.yaml-cpp
submodule (to fix warnings related to C++17)warnings.warn
pytest
test suiteChecklist
scons build
&scons test
) and unit tests address code coverage