8000 Add negative index to PixelAccess by Nazime · Pull Request #3406 · python-pillow/Pillow · GitHub
[go: up one dir, main page]
Skip to content

Add negative index to PixelAccess#3406

Merged
hugovk merged 3 commits intopython-pillow:masterfrom
Nazime:negativeindex
Oct 27, 2018
Merged

Add negative index to PixelAccess#3406
hugovk merged 3 commits intopython-pillow:masterfrom
Nazime:negativeindex

Conversation

@Nazime
Copy link
Contributor
@Nazime Nazime commented Oct 14, 2018 •
edited by hugovk
Loading

Add a negative indexing to the PixelAccess so we can get the last pixels easily

from PIL import Image

im = Image.open("image.png")
pix = im.load()
print(pix[-1, -1])

hugovk
hugovk requested changes Oct 14, 2018
View reviewed changes
Copy link
Member
@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Please could you add unit tests?

(I'm a little surprised this new code has coverage!)

Are these changes needed in both the Python and C layers?

src/_imaging.c Outdated
if (x < 0){
x = im->xsize + x;
}
if (y < 0){
Copy link
Member
@hugovk hugovk Oct 14, 2018

Choose a reason for hiding this comment

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

Please add a space between ) and {, here and above.

src/PIL/PyAccess.py Outdated
(x, y) = xy
if (x < 0):
x = self.xsize + x
if (y < 0):
Copy link
Member
@hugovk hugovk Oct 14, 2018

Choose a reason for hiding this comment

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

Please remove redundant parentheses, here and above.

hugovk
hugovk reviewed Oct 14, 2018
View reviewed changes
hugovk
hugovk reviewed Oct 14, 2018
View reviewed changes
@Nazime
Copy link
Contributor Author
Nazime commented Oct 14, 2018

I will add unit tests.

I don't know if the changes are needed in both C and python (I don't know how they are connected, I am new to Pillow), When i changed it in the Python code it didn't work.

@radarhere radarhere changed the title add negative index to PixelAccess Add negative index to PixelAccess Oct 15, 2018
@radarhere
Copy link
Member
radarhere commented Oct 15, 2018

Thanks for this.

You’ve added getting the negative index - it would be good to also add setting.

im.load() returns either PyAccess (Python) or PixelAccess (C). So that’s why you are finding that you need to change the C code.

@Nazime
Copy link
Contributor Author
Nazime commented Oct 15, 2018

What do you mean by add setting?

@radarhere
Copy link
Member
radarhere commented Oct 15, 2018

You have added support for print(pix[-1, -1]), but not pix[-1, -1] = (0, 0, 0) - the first being 'get', the second being 'set'.

@Nazime
Copy link
Contributor Author
Nazime commented Oct 15, 2018

Oh yes, I'll add that too.

@Nazime
86324a8 
Add setting negative pixels and unittest
radarhere
radarhere reviewed Oct 15, 2018
View reviewed changes
@Nazime
63a0b80 
misspelled variable name
@hugovk
Copy link
Member
hugovk commented Oct 17, 2018 •
edited
Loading

Let's remember to add this to the release notes.
8000

@hugovk hugovk merged commit e12aa37 into python-pillow:master Oct 27, 2018
@hugovk hugovk mentioned this pull request Oct 27, 2018
Closed
20 tasks
radarhere added a commit to radarhere/Pillow that referenced this pull request Nov 5, 2018
@radarhere
Added release notes for python-pillow#3406 [ci skip]
6039e54
radarhere added a commit to radarhere/Pillow that referenced this pull request Nov 5, 2018
@radarhere
Added release notes for python-pillow#3406 [ci skip]
cd93a6a
radarhere added a commit to radarhere/Pillow that referenced this pull request Nov 5, 2018
@radarhere
Added release notes for python-pillow#3406 [ci skip]
1ace2f4
hugovk added a commit that referenced this pull request Nov 5, 2018
@hugovk
Merge pull request #3451 from radarhere/releasenotes
3d76a68 
Added release notes for #3406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@radarhere radarhere radarhere left review comments

@hugovk hugovk hugovk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Nazime @radarhere @hugovk
0