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

fixed slicing of SolutionArray #816

Merged
merged 1 commit into from Mar 26, 2020
Merged

fixed slicing of SolutionArray #816

merged 1 commit into from Mar 26, 2020

Conversation

sin-ha
Copy link
Contributor

@sin-ha sin-ha commented Mar 4, 2020

Checklist

  • There is a clear use-case for this code change
  • The commit message has a short title & references relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • The pull request is ready for review

If applicable, fill in the issue number this pull request is fixing

Fixes #804
The changes allow list slicing in SolutionArray

Changes proposed in this pull request

-Changes to accommodate list slicing by making two cases of the SolutionArray

@sin-ha sin-ha changed the title fix for #804 fixed slicing of SolutionArray Mar 4, 2020
@ischoegl
Copy link
Member

ischoegl commented Mar 4, 2020

Also see 2f8f685 😉

@bryanwweber
Copy link
Member

@ischoegl Can you clarify if that is part of a particular PR or is it merged to master? If merged to master, why is #804 still open? Thanks

@ischoegl
Copy link
Member

ischoegl commented Mar 4, 2020

@bryanwweber ... the commit I referenced is part of #805, which is awaiting review. I stumbled across this while working on #805 and submitted an issue then.

I don't have a problem with @sin-ha's resolution, as it is virtually identical (#805 does depend on slicable SolutionArrays though, so the fix should be applied first in whichever way).

@sin-ha
Copy link
Contributor Author

sin-ha commented Mar 4, 2020

hey, @ischoegl @bryanwweber I am eager to work on related problems can you guide me? on #805

@ischoegl
Copy link
Member

ischoegl commented Mar 4, 2020

@sin-ha ... pull request #805 is currently awaiting review by one of the developers, so there's nothing to do there apart from reviewing.

For things to work on, the issue tracker is a good start. Look for open issues that do not have a linked pull request yet (you should see an icon on the overview; linked pull requests contain proposed fixes). You probably already read through the contributor's guide.

@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #816 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #816   +/-   ##
=======================================
  Coverage   71.39%   71.39%           
=======================================
  Files         372      372           
  Lines       43482    43482           
=======================================
  Hits        31045    31045           
  Misses      12437    12437           

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 24599b8...678a432. Read the comment docs.

@speth speth merged commit 5e23dec into Cantera:master Mar 26, 2020
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.

Slicing of SolutionArray raises error
4 participants