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

Some Transport updates #1327

Merged
merged 3 commits into from Jul 3, 2022
Merged

Some Transport updates #1327

merged 3 commits into from Jul 3, 2022

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jun 8, 2022

Changes proposed in this pull request

The header TransportBase.h does not match the naming convention used for other fundamental headers, while Transport.h does (see, e.g. includes in core.h introduced in #1238)

#include "cantera/base/Solution.h"
#include "cantera/thermo/ThermoPhase.h"
#include "cantera/kinetics/Kinetics.h"
#include "cantera/transport/TransportBase.h"

Also, streamline construction of Transport objects in Python.

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 June 8, 2022 01:24
@ischoegl
Copy link
Member Author

ischoegl commented Jun 8, 2022

@speth / @bryanwweber ... this is a trivial PR - not sure how to avoid the apparent 'churn' (which really isn't there) due to renaming + recreating a rudimentary file with the old name for deprecation purposes. I tried this in separate commits before, but it doesn't appear to make any difference ...

@codecov
Copy link

codecov bot commented Jun 8, 2022

Codecov Report

Merging #1327 (b69c989) into main (dceacdf) will increase coverage by 0.85%.
The diff coverage is 27.02%.

@@            Coverage Diff             @@
##             main    #1327      +/-   ##
==========================================
+ Coverage   67.19%   68.04%   +0.85%     
==========================================
  Files         314      314              
  Lines       41905    42007     +102     
  Branches    16862    16880      +18     
==========================================
+ Hits        28156    28582     +426     
+ Misses      11518    11167     -351     
- Partials     2231     2258      +27     
Impacted Files Coverage Δ
include/cantera/cython/wrappers.h 85.15% <ø> (ø)
include/cantera/transport/DustyGasTransport.h 0.00% <ø> (ø)
include/cantera/transport/GasTransport.h 100.00% <ø> (ø)
include/cantera/transport/TransportFactory.h 60.00% <ø> (-20.00%) ⬇️
include/cantera/transport/WaterTransport.h 50.00% <ø> (ø)
src/base/Solution.cpp 81.50% <ø> (ø)
src/base/YamlWriter.cpp 98.31% <ø> (ø)
src/clib/ctonedim.cpp 0.00% <ø> (ø)
src/oneD/IonFlow.cpp 50.00% <ø> (ø)
src/oneD/StFlow.cpp 87.54% <ø> (ø)
... and 36 more

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

@ischoegl ischoegl removed the request for review from a team June 8, 2022 02:01
@ischoegl ischoegl marked this pull request as draft June 8, 2022 02:01
@ischoegl ischoegl changed the title [Transport] Rename TransportBase.h to Transport.h Some Transport updates Jun 8, 2022
@ischoegl ischoegl marked this pull request as ready for review June 8, 2022 03:00
@ischoegl ischoegl requested a review from a team June 8, 2022 03:01
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 for fixing this annoying inconsistency, @ischoegl. I had just a few minor suggestions.

src/transport/Transport.cpp Show resolved Hide resolved
include/cantera/transport/Transport.h Outdated Show resolved Hide resolved
interfaces/cython/cantera/base.pyx Outdated Show resolved Hide resolved
@ischoegl
Copy link
Member Author

ischoegl commented Jul 3, 2022

@speth ... thanks for the review! Changes are taken care of.

@ischoegl ischoegl requested a review from speth July 3, 2022 08:19
@speth speth merged commit 0925fc0 into Cantera:main Jul 3, 2022
@ischoegl ischoegl deleted the rename-transportbase branch July 3, 2022 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants