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

Update composite.py #1086

Merged
merged 1 commit into from
Aug 24, 2021
Merged

Update composite.py #1086

merged 1 commit into from
Aug 24, 2021

Conversation

stijn76
Copy link
Contributor

@stijn76 stijn76 commented Aug 19, 2021

use a list object instead of range object for SolutionArray._indices, to continue using the append function with Python 3 (which has no longer an append function for range objects).

Changes proposed in this pull request

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

Closes #1085

If applicable, provide an example illustrating new features this pull request is introducing

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

use a list object instead of range object for SolutionArray._indices, to continue using the append function with Python 3 (which has no longer an append function for range objects).
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

@stijn76 ... thank you for submitting, and congratulations for your first PR! 🎉!

@decaluwe
Copy link
Member

Now you can make your second PR: adding your name to the AUTHORS file. 😁

@ischoegl
Copy link
Member

Now you can make your second PR: adding your name to the AUTHORS file. 😁

@decaluwe has a point! But it's not too late to add this to this PR as it's not merged yet. Also, feel free to check the tick boxes in the PR description.

@ischoegl
Copy link
Member

@stijn76 ... I wanted to briefly follow up. What @decaluwe referred to is the AUTHORS file in the main folder of the repository (which is a list of code contributors in alphabetical order). Feel free to add yourself, add the change to this PR, and I can merge.

@stijn76
Copy link
Contributor Author

stijn76 commented Aug 24, 2021

@ischoegl and @decaluwe : Thank you for the invitation to add my name to the authors file! I’ll take better care next time, also to add more comments in the bullet list of the pull request. For now, I’m okay with this; feel free to merge as is.

@ischoegl ischoegl merged commit 703fd1d into Cantera:main Aug 24, 2021
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.

SolutionArray.append sometimes fails
3 participants