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

Added support for encoding and decoding iTXt chunks. #818

Merged
merged 5 commits into from Jul 26, 2014

Conversation

dolda2000
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c469dd9 on dolda2000:itxt into * on python-pillow:master*.

@hugovk
Copy link
Member

hugovk commented Jul 23, 2014

@dolda2000 Please could you also add tests for this code? Thanks!

@dolda2000
Copy link
Contributor Author

@hugovk Not sure. When I run "setup.py test", it gives me an error:

Traceback (most recent call last):
  File "setup.py", line 753, in <module>
    zip_safe=True,
  File "/usr/lib/python3.2/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/lib/python3.2/distutils/dist.py", line 917, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python3.2/distutils/dist.py", line 936, in run_command
    cmd_obj.run()
  File "/usr/lib/python3/dist-packages/setuptools/command/test.py", line 137, in run
    self.with_project_on_sys_path(self.run_tests)
  File "/usr/lib/python3/dist-packages/setuptools/command/test.py", line 117, in     with_project_on_sys_path
    func()
  File "/usr/lib/python3/dist-packages/setuptools/command/test.py", line 146, in run_tests
    testLoader = loader_class()
  File "/usr/lib/python3.2/unittest/main.py", line 123, in __init__
    self.parseArgs(argv)
  File "/usr/lib/python3.2/unittest/main.py", line 191, in parseArgs
    self.createTests()
  File "/usr/lib/python3.2/unittest/main.py", line 198, in createTests
    self.module)
  File "/usr/lib/python3.2/unittest/loader.py", line 137, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib/python3.2/unittest/loader.py", line 137, in <listcomp>
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib/python3.2/unittest/loader.py", line 105, in loadTestsFromName
    parent, obj = obj, getattr(obj, part)
AttributeError: 'module' object has no attribute 'tests'

Is this not how one is supposed to run tests on PIL? I will admit I have no experience with Python's testing framework.

@hugovk
Copy link
Member

hugovk commented Jul 23, 2014

@dolda2000 You can run them all like this (taken from .travis.yml):

nosetests -vx Tests/test_*.py

Or with coverage:

coverage run --append --

include=PIL/* -m nose -vx Tests/test_*.py

Or to run just a single test:

python Tests/test_sometest.py

Either edit an appropriate existing test file or create a new one if you wish; helper.py has a base test class with helper functions. Test images go in Tests/images/.

@dolda2000
Copy link
Contributor Author

Thanks.

I added some tests. Is that something you would be happy with?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2b4d91e on dolda2000:itxt into * on python-pillow:master*.

@hugovk
Copy link
Member

hugovk commented Jul 23, 2014

@dolda2000 Yes, that looks the right sort of thing. Please can you also test the new add_itxt() and the iTxt part of add_text()?

@dolda2000
Copy link
Contributor Author

Ah, right, of course. Fixed.

I also took the opportunity to do it so that Unicode strings with non-Latin1 characters in them are always encoded as iTXt chunks, in order to actually preserve their contents.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2687b5c on dolda2000:itxt into * on python-pillow:master*.

@@ -329,6 +362,43 @@ def chunk_zTXt(self, pos, length):
self.im_info[k] = self.im_text[k] = v
return s

def chunk_iTXt(self, pos, length):
Copy link
Member

Choose a reason for hiding this comment

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

This function uses a lot of single-letter variable names and it's not always clear what they do. Please could you use more descriptive names?

@dolda2000
Copy link
Contributor Author

To be fair, I was trying to keep in style with the surrounding chunk_tEXt and chunk_zTXt functions. :)

Since they use similar variable naming, I don't quite see what would be better.

hugovk added a commit that referenced this pull request Jul 26, 2014
Added support for encoding and decoding iTXt chunks.
@hugovk hugovk merged commit 328fd35 into python-pillow:master Jul 26, 2014
@hugovk
Copy link
Member

hugovk commented Jul 26, 2014

Thanks for the contribution!

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.

None yet

3 participants