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

ENH: Add numbers to the three itkPyBufferMemoryLeakTest print messages #2578

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Jun 4, 2021

Add numbers (1, 2, 3) to the print messages in itkPyBufferMemoryLeakTest,
to ease analysis of the output from a test failure.

@github-actions github-actions bot added area:Bridge Issues affecting the Bridge module area:Python wrapping Python bindings for a class type:Enhancement Improvement of existing methods or implementation type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Jun 4, 2021
Add numbers (1, 2, 3) to the print messages in itkPyBufferMemoryLeakTest,
to ease analysis of the output from a test failure.
@N-Dekker N-Dekker force-pushed the Number-print-messages-itkPyBufferMemoryLeakTest branch from 941c4a2 to fbb5158 Compare June 4, 2021 20:44
@N-Dekker N-Dekker requested a review from thewtex June 4, 2021 20:50
@N-Dekker
Copy link
Contributor Author

N-Dekker commented Jun 4, 2021

@dzenanz Could the itkPyBufferMemoryLeakTest failures possibly be introduced by pull request #2554 "BUG: add support for long long pixel types in PyBuffer", merged by cd36c64 ?

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Incremental build didn't work for me, I needed to manually rebuild something to avoid a clean build.

While it is possible that PR did it, I see no reason why it would do that. This same test used to be flaky on Mac before, if I remember correctly.

@N-Dekker
Copy link
Contributor Author

N-Dekker commented Jun 4, 2021

While it is possible that PR did it, I see no reason why it would do that.

I don't know either. Would it matter that these types from itkPyBuffer and itkImageDuplicator have become out of sync, with PR #2554 ?

UNIQUE(types "${WRAP_ITK_SCALAR};UC;D;US;UI;UL;ULL;SC;SS;SI;SL;SLL;F")

# Making sure that all types wrapped in PyBuffer are also wrapped for ImageDuplicator
# as this filter is used in the Python NumPy bridge.
UNIQUE(types "${WRAP_ITK_SCALAR};UC;D;US;UI;UL;SC;SS;SI;SL;F")

@N-Dekker N-Dekker marked this pull request as ready for review June 5, 2021 08:02
@dzenanz
Copy link
Member

dzenanz commented Jun 7, 2021

Even if it does not fix the pyBuffer memory leak, ImageDuplicator should be wrapped with LL types. -> Done in #2584.

@dzenanz dzenanz merged commit 8d2de71 into InsightSoftwareConsortium:master Jun 7, 2021
dzenanz added a commit that referenced this pull request Jun 7, 2021
thewtex pushed a commit that referenced this pull request Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Bridge Issues affecting the Bridge module area:Python wrapping Python bindings for a class type:Enhancement Improvement of existing methods or implementation type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants