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

Replace stylevalue 'setlinewidth' together with style attribute by 'penwidth' #463

Merged
merged 1 commit into from
Jul 11, 2017

Conversation

tilmanbremer
Copy link

Changes proposed in this pull request:

@codecov
Copy link

codecov bot commented Jul 1, 2017

Codecov Report

Merging #463 into master will decrease coverage by <.01%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #463      +/-   ##
==========================================
- Coverage   58.23%   58.23%   -0.01%     
==========================================
  Files         386      386              
  Lines       42310    42309       -1     
  Branches     7252     7252              
==========================================
- Hits        24640    24639       -1     
  Misses      15488    15488              
  Partials     2182     2182
Impacted Files Coverage Δ
src/kinetics/ReactionPath.cpp 71.88% <57.14%> (-0.06%) ⬇️

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 6bf74d1...49aa58b. Read the comment docs.

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 the clear and clean PR. It looks like this does just what it needs to, and I don't see any problems except with ancient versions of Graphviz.

@speth speth merged commit b41038f into Cantera:master Jul 11, 2017
@tilmanbremer
Copy link
Author

I tried to keep it simple, because this was my first pull-request ever. Thanks for approving it. This was a result of my attempt to provide a better documentation for the reaction path diagram feature of cantera (https://www.tilmanbremer.de/2017/06/tutorial-generating-reaction-path-diagrams-with-cantera-and-python/). I will try to make more sophisticated commits to ReactionPath.cpp soon.

@santoshshanbhogue
Copy link
Contributor

santoshshanbhogue commented Jul 13, 2017 via email

@tilmanbremer
Copy link
Author

Thank you Santosh. I would like to contribute to the official docs, too, but I believe it makes more sense to improve the whole reaction path functionality first, and then update the docs accordingly. So the tutorial was a way for me to really dive into the current situation, now I hopefully can improve it from here.

@speth
Copy link
Member

speth commented Jul 16, 2017

The current reaction path code is certainly in need of a major overhaul, as well as better documentation. Any improvements in this area would certainly be welcome. One thing I've been meaning to look into but haven't had the time is how RMG generates its flux diagrams, which are very nice looking compared to the clunky ones that Cantera currently generates (see for example the one on the rmg.mit.edu homepage).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants