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

refactoring + autoresize + multiple comparison #7

Merged
merged 7 commits into from
Feb 16, 2014

Conversation

koteth
Copy link

@koteth koteth commented Feb 12, 2014

I’ve done a small refactoring for myself.
I’ve also enabled the possibility to compare an image against a list (directory) of images.
if you pass 2 images as input the output is the same as the old code.
if you want to take my code is ok .

@jterrace
Copy link
Owner

I like the enhancements, but why did you move everything out of init and main ?

@koteth
Copy link
Author

koteth commented Feb 13, 2014

hi, init.py in python is mainly a marker that qualifies a directory as a package.
it's usually empty so it's the last place where I search for a block of code in a project. For this reason I prefer not to use it if is not needed.

Regarding main.py file, it is usually present if you want to run a program with 'python ', but the README file suggests to install the module. So, once it is correctly installed (in setup.py i've pointed to "pyssim = ssim.ssim:main'"), the main file is not needed anymore.

@jterrace
Copy link
Owner

init being empty is a commonly used convention, but you typically import anything you want to be globally accessible from your module into it. I can't merge this because you broke compatibility with anyone using the module like this:

import ssim
ssim.compute_ssim(...)

The main file allows you to do this:

python -m ssim

or from developemt:

python ssim

@koteth
Copy link
Author

koteth commented Feb 15, 2014

you are right, I have made some small backward compatibility enhancements in order to support the 3 scenarios you have explained.
Now it should be ok.

index = numpy.average(ssim_map)

return index
def compute_ssim(image1, image2, gaussian_kernel_sigma=1.5, gaussian_kernel_width=11):
Copy link
Owner

Choose a reason for hiding this comment

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

Pelase copy the original docstring back into this function

added import of SSIM and SSIMImage in __init__.py
@koteth
Copy link
Author

koteth commented Feb 16, 2014

done

jterrace added a commit that referenced this pull request Feb 16, 2014
refactoring + autoresize + multiple comparison
@jterrace jterrace merged commit d9f92ea into jterrace:master Feb 16, 2014
@jterrace
Copy link
Owner

Thanks for your contribution!

@jterrace
Copy link
Owner

Hey, would you mind sending me a PR to add yourself to the AUTHORS file?

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