Skip to content

Paganin filter corrections #547

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Paganin filter corrections #547

wants to merge 4 commits into from

Conversation

@Arhatari
Copy link

@Arhatari Arhatari commented Jun 11, 2021 •
edited by carterbox
Loading

Paganin's filter is corrected according to the equation in the original paper.
Frequency domain is normalized to the value of 2*pi instead of original value of 0.5.
When padding is set to False, the result was giving constant value on the image. Therefore the code is changed accordingly.
Minus logarithm is added according to the Paganin's filter equation. (The place of the minus logarithm could also be in the filter itself).

Closes #474 Related to #500

@pep8speaks
Copy link

pep8speaks commented Jun 11, 2021 •
edited
Loading

Hello @Arhatari! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 198:61: W291 trailing whitespace

Comment last updated at 2021-08-24 02:01:43 UTC

@carterbox carterbox requested a review from dgursoy June 11, 2021 18:40
@dgursoy
Copy link
Collaborator

dgursoy commented Jun 15, 2021

Thanks @Arhatari for this PR! This has been brought up several times before, but never had the chance to do it properly.

However, because there are many users that may have used and become familiar with the parameter settings of the "current" filter, I suggest we name the new function as retrieve_phase_paganin so that some of the users want to continue using their old function. We may need to update the documentation and say which what each function actually does. What do you say?

@Arhatari
Copy link
Author

Arhatari commented Jun 16, 2021 via email •
edited by carterbox
Loading

@github-actions
Copy link

github-actions bot commented Aug 18, 2021

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale Issues without activity in the past 60 days label Aug 18, 2021
@Arhatari Arhatari changed the title Paganin filter correction Paganin filter corrections Aug 24, 2021
@github-actions github-actions bot removed the stale Issues without activity in the past 60 days label Aug 24, 2021
@github-actions
Copy link

github-actions bot commented Oct 23, 2021

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale Issues without activity in the past 60 days label Oct 23, 2021
@dkazanc dkazanc mentioned this pull request Sep 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dgursoy dgursoy Awaiting requested review from dgursoy

Assignees

@Arhatari Arhatari

Labels

stale Issues without activity in the past 60 days

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TIE-Hom in Tomopy

3 participants

@Arhatari @pep8speaks @dgursoy