Gramps Bugtracker - Gramps 2.2.X
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001255Gramps 2.2.XMediapublic2007-09-27 06:252007-09-28 03:56
Reportertgehrig 
Assigned To 
PrioritynormalSeverityfeatureReproducibilityalways
StatusclosedResolutionwon't fix 
PlatformOSOS Version
Summary0001255: use thumbnails of subsection instead of the whole image in person specific views
DescriptionI attached a patch against 2.2.8 to use the subsections specified in the
MediaRef for the thumbnail generation in the person editor, relationship
view and pedigree view. I also added a preview to the MediaRef dialog to
see the actual subsection. Sadly I am not much of an gui guru, so there
is no interactive rectangle selection inside the image, but at least the
thumbnail updates as the spinbuttons' values are changed.
It is more or less a quick hack, because I wanted to get this working
without digging too deep into the code;-).
The patch seems to also apply cleanly against the gramps22 svn branch.
But for 3.0 there are some small changes necessary, because the
thumbnail generation seems to have moved from ImgManip.py to
ThumbNails.py. I will post this soon.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
related to 0002513new  Feature Requests Using section/region on media_ref as thumbnail on reports 
Attached Filesbz2 gramps-2.2.8-subsection3.patch.bz2 (3,776) 2007-09-27 06:31
http://www.gramps-project.org/bugs/
Issue History
Date ModifiedUsernameFieldChange
2007-09-27 06:25tgehrigNew Issue
2007-09-27 06:25tgehrigFile Added: gramps-2.2.8-subsection2.patch.bz2
2007-09-27 06:31tgehrigFile Added: gramps-2.2.8-subsection3.patch.bz2
2007-09-27 06:32tgehrigNote Added: 0003059
2007-09-27 08:37bmcageNote Added: 0003062
2007-09-27 09:06tgehrigNote Added: 0003063
2007-09-27 09:57bmcageFile Deleted: gramps-2.2.8-subsection2.patch.bz2
2007-09-27 09:58bmcageNote Added: 0003066
2007-09-27 10:50tgehrigNote Added: 0003069
2007-09-27 11:06bmcageNote Added: 0003071
2007-09-27 11:23tgehrigNote Added: 0003072
2007-09-27 16:32bmcageNote Added: 0003074
2007-09-27 18:25tgehrigNote Added: 0003080
2007-09-28 03:56bmcageStatusnew => closed
2007-09-28 03:56bmcageNote Added: 0003091
2007-09-28 03:56bmcageResolutionopen => won't fix
2009-07-04 05:43romjeromeRelationship addedrelated to 0002513

Notes
(0003059)
tgehrig   
2007-09-27 06:32   
I removed the debian part of the patch in gramps-2.2.8-subsection3.patch.bz2
(0003062)
bmcage   
2007-09-27 08:37   
Tobias, can you delete the patch that is not ok anymore? It get's confusing otherwise.

If you make a patch for 3.0 I would probably also apply the 2.2.8 patch, if ok with Brian.

Nice patch, did not apply it, but the code looks good to me. Some quick comments nevertheless:
1/in ImgManip.py, I see twice
 w=pixbuf.get_width()
 h=pixbuf.get_heigth()
Is that needed?

2/don't give parameter if they are named and same as default value.
So not
ImgManip.get_thumbnail_path(obj.get_path(), None, ph.get_rectangle())
but
ImgManip.get_thumbnail_path(obj.get_path(), rectangle=ph.get_rectangle())
Same in relationview

3/documentation strings. I know you add stuff to some modules that have no or little comment strings, but nevertheless, new code should have it. So every method, class, function, module you add should have the
""" documentation
"""
section. Sorry! Eg in your set_uppery I don't know from reading code if left or right corner.

4/You should not use the type function. See http://www.python.org/dev/peps/pep-0008/ [^]
So you should use IsInstance(Obj,tuple). Anyway, is there a need here to check that? I understand the reasoning, but as you set rectangle yourself in the code as a tuple, if it exists it is a tupple, no? If somebody misuses rectangle, I would prefer an error that is grabbed by the exception handler, instead of the code that keeps working without people knowing something is wrong. Feel free to disagree on this.
(0003063)
tgehrig   
2007-09-27 09:06   
I would like to delete the first patch, but how do I do that? I don't find anything:-/
(0003066)
bmcage   
2007-09-27 09:58   
Ok, I supposed as uploader you could delete it yourself, apparently not.
I did it for you (Delete link is behind the filename for me)
(0003069)
tgehrig   
2007-09-27 10:50   
About your comments:
1.) is not really needed;-). As I said, it was a quick hack;-). I have fixed this here locally for 3.0. I will publish that patch soon. I don't want to do twice the work. So is it okay if I only update the patch for 3.0?
3.) Okay, I hate to do documentation, but I understand that it is necessary:-/.
4.) I just copied what was already in the code. But as you said normally rectangle should always be None or a tuple.
(0003071)
bmcage   
2007-09-27 11:06   
only patch for 3.0 is fine with me.
It is an extensive feature, so if you can life without it for 2.2.x (or only on your box), then only send in 3.0 patch
(0003072)
tgehrig   
2007-09-27 11:23   
Those that want this feature in 2.2.8 probably can also live with the current patch;-).
(0003074)
bmcage   
2007-09-27 16:32   
note that I need your name and email as it must appear in ChangeLog file.
Send it in the patch (eg in ChangeLog) or to me directly.
That is to contact you should we want to sell gramps ;-)
(0003080)
tgehrig   
2007-09-27 18:25   
I have just submitted a new ticket for 3.x here: http://bugs.gramps-project.org/view.php?id=1259 [^]
This should hopefully match your requirements.
(0003091)
bmcage   
2007-09-28 03:56   
We won't add this to 2.2.x, as that branch is in bug-fix only mode.

See issue 0001259 for the 3.0 patch that will be included.