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

Fix compilation warnings #1346

Merged
merged 4 commits into from
Aug 2, 2022
Merged

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jul 30, 2022

Changes proposed in this pull request

Fix compilation warnings caused by comparisons of int and size_t.

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

@ischoegl ischoegl requested a review from a team July 30, 2022 15:10
@codecov
Copy link

codecov bot commented Jul 30, 2022

Codecov Report

Merging #1346 (8ed4a6f) into main (ade1a38) will increase coverage by 0.00%.
The diff coverage is 81.81%.

@@           Coverage Diff           @@
##             main    #1346   +/-   ##
=======================================
  Coverage   68.15%   68.15%           
=======================================
  Files         327      327           
  Lines       42658    42659    +1     
  Branches    17180    17180           
=======================================
+ Hits        29074    29075    +1     
  Misses      11291    11291           
  Partials     2293     2293           
Impacted Files Coverage Δ
...e/cantera/zeroD/IdealGasConstPressureMoleReactor.h 100.00% <ø> (ø)
include/cantera/zeroD/MoleReactor.h 11.11% <ø> (ø)
src/thermo/Elements.cpp 88.00% <80.00%> (ø)
include/cantera/numerics/PreconditionerBase.h 25.92% <100.00%> (+2.84%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@ischoegl ischoegl closed this Jul 31, 2022
@ischoegl ischoegl reopened this Jul 31, 2022
@ischoegl ischoegl force-pushed the fix-compilation-warnings branch 2 times, most recently from 28c60c7 to 2dc89e9 Compare July 31, 2022 11:58
@ischoegl
Copy link
Member Author

ischoegl commented Jul 31, 2022

Alright - I fixed the warnings per discussion (together with two more I located, notably the mildly annoying NPY_1_7_API_VERSION warning generated by Cython).

Otherwise, I believe this is good to go from my side.

Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ischoegl. It's great to see that there is finally a fix for the Numpy/Cython deprecation warning, especially after #1334 made several more instances of it appear.

I just had one minor concern related to the changes to .gitignore.

.gitignore Outdated Show resolved Hide resolved
@ischoegl
Copy link
Member Author

ischoegl commented Aug 2, 2022

@speth ... thanks for the review! I dropped the .gitignore bit, so this should be good to go.

@ischoegl ischoegl requested a review from speth August 2, 2022 04:04
@speth speth merged commit 177356c into Cantera:main Aug 2, 2022
@ischoegl ischoegl deleted the fix-compilation-warnings branch August 2, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants