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

Windowing histogram #298

Merged
merged 4 commits into from
Oct 21, 2023
Merged

Conversation

danielhorton4001
Copy link
Contributor

In 3D view, it syncs to the Axial view.
The seperate views cannot support their own windowing sliders until the way windowing is handled is changed to accomodate individual slice windowing.

danielhorton4001 and others added 4 commits October 2, 2023 21:23
Histogram updates to selected slice.
In 3D view, it syncs to the Axial view.
The seperate views cannot support their own windowing sliders until the way windowing is handled is changed to accomodate individual slice windowing.
Copy link
Collaborator

@sjswerdloff sjswerdloff left a comment

Choose a reason for hiding this comment

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

I'd like a separate issue created to address the magic number.
I'll approve based on the assumption that will get fixed in a subsequent PR.

slices = len(self.pixel_values)
self.densities = [[]] * slices
for s in range(slices):
self.densities[s] = [0] * WindowingSlider.MAX_PIXEL_VALUE
Copy link
Collaborator

Choose a reason for hiding this comment

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

so your histogram has MAX_PIXEL_VALUE bins?


# Normalise values
for i in range(WindowingSlider.MAX_PIXEL_VALUE):
self.densities[s][i] = self.densities[s][i] / max_value * 10000
Copy link
Collaborator

Choose a reason for hiding this comment

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

10000 is a magic number. prefer refactor to constant or variable with meaningful name

@sjswerdloff sjswerdloff merged commit ed39a95 into didymo:master Oct 21, 2023
5 checks passed
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.

2 participants