Add write_hdf to SolutionArray objects by ischoegl · Pull Request #680 · Cantera/cantera

ischoegl · 2019-08-06T17:36:22Z

Please fill in the issue number this pull request is fixing:

Fixes #679

Changes proposed in this pull request:

The commit implements saving of data extracted from SolutionArrays to HDF containers using pandas infrastructure.
The method only works if the pandas module can be imported; an exception is only raised if the method is called without a working pandas installation.

A simple example for the intended use would be:

# create SolutionArray and/or run some manipulations
states = ct.SolutionArray(gas, 10)
...

states.write_hdf('some_file.h5')

codecov · 2019-08-06T18:00:06Z

Codecov Report

Merging #680 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   70.62%   70.62%   +<.01%     
==========================================
  Files         372      372              
  Lines       43565    43565              
==========================================
+ Hits        30768    30769       +1     
+ Misses      12797    12796       -1

Impacted Files	Coverage Δ
src/transport/GasTransport.cpp	`90.58% <0%> (+0.2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbdc790...f39e122. Read the comment docs.

bryanwweber · 2019-08-06T21:10:55Z

Does pandas require a particular HDF5 package? I don't think that's a default dependency of Pandas, but I could be wrong

ischoegl · 2019-08-06T21:30:30Z

@bryanwweber ... it uses PyTables as backend. I keep a pretty sparse build tool chain, and apt-get install python3-pandas took care of everything. Haven't checked pip yet, but can look into this.

ischoegl · 2019-08-06T21:36:52Z

PS: if this has legs, I am still planning on implementing unit tests, but wanted to get some feedback before getting into the CI settings.

bryanwweber

I'm definitely in favor of this change, just some minor comments. I also don't think there's much to test here, since all of the functionality is either tested elsewhere (collect_states()) or part of the Pandas package, which we don't really need to test.

I'm also interested in how a pip-installed or conda-installed Pandas manages the HDF5 dependency.

interfaces/cython/cantera/composite.py

bryanwweber · 2019-08-07T16:11:28Z

I forgot to mention in the review, there's a few places where pandas should be uppercase Pandas in the docstring.

ischoegl · 2019-08-07T17:05:32Z

Alright. pip dependencies require pandas and tables (i.e. Pandas and PyTables). Docstring is updated accordingly.

codecov · 2019-08-07T17:09:28Z

Codecov Report

Merging #680 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   70.62%   70.62%   +<.01%     
==========================================
  Files         372      372              
  Lines       43565    43565              
==========================================
+ Hits        30768    30769       +1     
+ Misses      12797    12796       -1

Impacted Files	Coverage Δ
src/transport/GasTransport.cpp	`90.58% <0%> (+0.2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbdc790...5c9e348. Read the comment docs.

ischoegl · 2019-08-07T17:11:40Z

Ugh. Last push was autopep'd. Can revert if so desired.

speth · 2019-08-07T17:15:38Z

Please do. It makes the intentional changes hard to see in the diff.

ischoegl · 2019-08-07T17:27:12Z

Please do. It makes the intentional changes hard to see in the diff.

done. thankfully the file was mostly compliant with autopep8's choices ;) ... unintentionally opened emacs from within a conda environment with an autopep8-on-save hook enabled.

speth

I'm in favor of this addition. My only suggested changes relate to the docstrings for the new methods.

interfaces/cython/cantera/composite.py

ischoegl · 2019-08-09T20:59:44Z

@speth ... I just noticed that two issues you wanted to have addressed didn't make it into the last commit (I had taken care of it but the commit was lost). It is now fixed.

ischoegl · 2019-08-09T21:34:15Z

@bryanwweber ... if this looks good to you, could you approve so I can add HDF support to #687?

* The commit implements saving of data extracted from SolutionArrays to HDF containers using pandas infrastructure. * Two methods are introduced: `write_hdf` and `to_pandas`. * Both methods only work if the pandas module can be imported; an exception is raised only if the method is called without a working pandas installation.

codecov · 2019-08-09T22:22:52Z

Codecov Report

Merging #680 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   70.62%   70.63%   +<.01%     
==========================================
  Files         372      372              
  Lines       43575    43575              
==========================================
+ Hits        30777    30778       +1     
+ Misses      12798    12797       -1

Impacted Files	Coverage Δ
src/transport/GasTransport.cpp	`90.58% <0%> (+0.2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97356a4...9f76dec. Read the comment docs.

codecov · 2019-08-09T22:22:52Z

Codecov Report

Merging #680 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   70.62%   70.63%   +<.01%     
==========================================
  Files         372      372              
  Lines       43575    43575              
==========================================
+ Hits        30777    30778       +1     
+ Misses      12798    12797       -1

Impacted Files	Coverage Δ
src/transport/GasTransport.cpp	`90.58% <0%> (+0.2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97356a4...9f76dec. Read the comment docs.

ischoegl force-pushed the add-write-hdf-to-solution-array branch 3 times, most recently from dec7fa2 to f39e122 Compare August 6, 2019 18:00

bryanwweber requested changes Aug 7, 2019

View reviewed changes

interfaces/cython/cantera/composite.py Outdated Show resolved Hide resolved

interfaces/cython/cantera/composite.py Outdated Show resolved Hide resolved

ischoegl force-pushed the add-write-hdf-to-solution-array branch from f39e122 to 99f3540 Compare August 7, 2019 17:05

ischoegl force-pushed the add-write-hdf-to-solution-array branch from 99f3540 to 5c9e348 Compare August 7, 2019 17:09

ischoegl force-pushed the add-write-hdf-to-solution-array branch from 5c9e348 to ffa4d3d Compare August 7, 2019 17:23

ischoegl force-pushed the add-write-hdf-to-solution-array branch 2 times, most recently from fb802e2 to d4f4eeb Compare August 7, 2019 20:07

speth requested changes Aug 7, 2019

View reviewed changes

interfaces/cython/cantera/composite.py Outdated Show resolved Hide resolved

interfaces/cython/cantera/composite.py Outdated Show resolved Hide resolved

interfaces/cython/cantera/composite.py Outdated Show resolved Hide resolved

ischoegl force-pushed the add-write-hdf-to-solution-array branch from d4f4eeb to 99eb201 Compare August 7, 2019 22:33

This was referenced Aug 8, 2019

SolutionArray save / load #543

Closed

Implement recreation of SolutionArray objects from stored data #687

Merged

ischoegl force-pushed the add-write-hdf-to-solution-array branch 2 times, most recently from 3eb9e28 to 7378cbc Compare August 9, 2019 20:57

speth approved these changes Aug 9, 2019

View reviewed changes

ischoegl force-pushed the add-write-hdf-to-solution-array branch from 7378cbc to d1f1f80 Compare August 9, 2019 21:55

ischoegl force-pushed the add-write-hdf-to-solution-array branch from d1f1f80 to 9f76dec Compare August 9, 2019 21:57

bryanwweber approved these changes Aug 13, 2019

View reviewed changes

speth merged commit eea0425 into Cantera:master Aug 13, 2019

ischoegl deleted the add-write-hdf-to-solution-array branch October 5, 2019 00:11

ischoegl mentioned this pull request Mar 26, 2020

SolutionArray/HDF interface for 1D FlameBase #805

Merged

4 tasks

Uh oh!

Conversation

ischoegl commented Aug 6, 2019

Uh oh!

codecov bot commented Aug 6, 2019

Codecov Report

Uh oh!

bryanwweber commented Aug 6, 2019

Uh oh!

ischoegl commented Aug 6, 2019

Uh oh!

ischoegl commented Aug 6, 2019

Uh oh!

bryanwweber left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bryanwweber commented Aug 7, 2019

Uh oh!

ischoegl commented Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 7, 2019

Codecov Report

Uh oh!

ischoegl commented Aug 7, 2019

Uh oh!

speth commented Aug 7, 2019

Uh oh!

ischoegl commented Aug 7, 2019

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ischoegl commented Aug 9, 2019

Uh oh!

ischoegl commented Aug 9, 2019

Uh oh!

codecov bot commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov bot commented Aug 9, 2019

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bryanwweber left a comment •

edited

Loading

ischoegl commented Aug 7, 2019 •

edited

Loading

codecov bot commented Aug 9, 2019 •

edited

Loading