Closed moritz-h closed 3 years ago
Merging #28 (123185c) into master (f7b5b77) will increase coverage by
0.17%
. The diff coverage is81.25%
.
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
+ Coverage 70.32% 70.50% +0.17%
==========================================
Files 6 6
Lines 829 834 +5
==========================================
+ Hits 583 588 +5
Misses 246 246
Impacted Files | Coverage Δ | |
---|---|---|
pyevtk/vtk.py | 85.50% <70.00%> (+0.07%) |
:arrow_up: |
pyevtk/hl.py | 80.69% <100.00%> (+0.30%) |
:arrow_up: |
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 f7b5b77...123185c. Read the comment docs.
Thank you for the PR @moritz-h! Could you change the type().__name__
checks to use isinstance
instead? Otherwise I think this can be merged directly and also warrants a new release.
@renefritze I copied the check from within vtkFile.addData(), which is called directly after: https://github.com/pyscience-projects/pyevtk/blob/v1.2.0/pyevtk/vtk.py#L533 I think it would make sense to keep this consistent. Do you prefer to update update both places then or should I avoid changes within vtk.py?
@renefritze I copied the check from within vtkFile.addData(), which is called directly after: https://github.com/pyscience-projects/pyevtk/blob/v1.2.0/pyevtk/vtk.py#L533
I think it would make sense to keep this consistent. Agreed. Do you prefer to update update both places then or should I avoid changes within vtk.py? I would prefer updating both places, if you don't mind. Maybe the
ndarray
check too?
@renefritze Should be done
I'll have to look into the apparently broken GH action before I do the next release, and it might be a few days before I get to that.
It looks like an authentication issue to pypi. I think the reason might be that third party pull request do not have access to the secret defined in the GitHub workflow. (Which is generally a very good idea from security perspective.)
It looks like an authentication issue to pypi. I think the reason might be that third party pull request do not have access to the secret defined in the GitHub workflow. (Which is generally a very good idea from security perspective.)
Yeah, my issue is only that I remembered having that setup so it at least wouldn't fail the PR check.
I think this line may is wrong: https://github.com/pyscience-projects/pyevtk/blob/master/.github/workflows/test_deploy.yml#L6 The deploy test may only should run when it is merged into master and not onto pull requests.
While wanting to use a static URL above, I noticed, when switching back to v1.2.0, there this line is exactly like what my idea would have been: https://github.com/pyscience-projects/pyevtk/blob/v1.2.0/.github/workflows/test_deploy.yml
Maybe this change to test_deploy.yml within this commit was unintentional and it has worked before: https://github.com/pyscience-projects/pyevtk/commit/571426c4272a93c756b6aae0b8b27bed5cbde160#diff-6b5ff66c8969e24f75b0e7d96ffd86e0939f2e7ef4ed90658d6c054fafcee1fa
You are absolutely right, that is an entirely unrelated and weird change in 571426c. Good catch! Thanks!
1.3.0 is now out with your fixes included @moritz-h. Thanks again!
Fix #27.
addData()