My first python plugin review - Printable Version

My first python plugin review - Printable Version

+- Gimp-Forum.net (https://www.gimp-forum.net)
+-- Forum: GIMP (https://www.gimp-forum.net/Forum-GIMP)
+--- Forum: Extending the GIMP (https://www.gimp-forum.net/Forum-Extending-the-GIMP)
+---- Forum: Scripting questions (https://www.gimp-forum.net/Forum-Scripting-questions)
+---- Thread: My first python plugin review (/Thread-My-first-python-plugin-review)

Pages: 1 2

RE: My first python plugin review - origamifreak2 - 01-01-2025

I've updated the code to handle pretty much any aspect ratio, and I added some standard aspect ratios as defaults.

https://github.com/origamifreak2/Gimp-Plugins/blob/main/change_aspect_ratio/change_aspect_ratio.py

I actually took a look at one of your plugins @Ofnuts to see how to register multiple plugins under the same submenu. Let me know if there's anything I can improve Smile

RE: My first python plugin review - Ofnuts - 01-01-2025

(01-01-2025, 12:39 AM)origamifreak2 Wrote: Thanks Ofnuts . I updated my code. I also added some logic to convert the image to RGB mode if it's indexed.

This isn't something I would do. I would only enable the filter on RGB* and GRAYSCALE*. Then if someone really wants to use the filter on an indexed image, de-indexing it beforehand is a quick single-click away. Because now your filter needs to re-index the image, and how to do so properly is a nice can of worms (how many colors? which palette?).

RE: My first python plugin review - origamifreak2 - 01-06-2025

Thanks again Ofnuts

I took your suggestion. I've also made a ton of other changes to the code.

added ability to fill background with the active foreground color
added ability to fill background with the active background color
added ability to fill background with a different user selected color
added ability to fill background with transparency
added ability for user to select a specific blur radius between 0 and 500

The code is quite a bit more complicated now, but please feel free to take a look and let me know if anything could be done better.

RE: My first python plugin review - Ofnuts - 01-06-2025

elif(background_type == 1 or background_type == 2 or background_type == 3) better written as if background_type in [1,2,3]

elif(background_type == 4): # transparent background
# do nothing since resized canvas background is already transparent
pass

better written as
else: # transparent background case: do nothing since resized canvas background is already transparent
pass
(always better to have an explicit else to catch what we could have forgotten)

Instead of saving the color, you can bracket your code with gimp.context_push()/gimp.context_pop() (like the undo group), so that any context change you make for the script is reset when the script exits.

Blur radius can be a PF_SLIDER, easier for the user and since you add value boundaries you naturally limit the user to what Gimp's blur allows.

When you use pre-defined values as parameters, use their name, for instance pdb.gimp_image_merge_down(image, drawable, 1) is better written pdb.gimp_image_merge_down(image, drawable,CLIP_TO_IMAGE ) (note the "-/_" substitution).

RE: My first python plugin review - origamifreak2 - 01-08-2025

I believe I've implemented all your suggestions @Ofnuts. Thanks again. Let me know if there's anything else that could be improved.

RE: My first python plugin review - Ofnuts - 01-08-2025

The idea of context_push()/Context_pop() is that they bracket the whole code (like the undo_group thing) , so that once you have set it up your code can alter the context without having to worry about it.

Beyond this it becomes a matter of style. Personally, I loathe if/else, because this tends to create code duplicates(*) so you have to maintain two or more branches in parallel. So my way of writing this:

Code:

       if(background_type == 1): # foreground color

            pdb.gimp_edit_bucket_fill(background, BUCKET_FILL_FG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1)

        elif(background_type == 2): # background color

            pdb.gimp_edit_bucket_fill(background, BUCKET_FILL_BG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1)

        elif(background_type == 3): # other color

            pdb.gimp_context_push()

            pdb.gimp_context_set_background(background_color) # set new background color

            pdb.gimp_edit_bucket_fill(background, BUCKET_FILL_BG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1) # fill background with new color

            pdb.gimp_context_pop()

would be:

Code:

       gimp.context_push()

        # most of your code goes here

        color=[gimp.get_foreground(),gimp.get_background(),background_color][background_type-1]

        pdb.gimp_edit_bucket_fill(color, BUCKET_FILL_BG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1) # fill background with new color

        # some more of your code goes here

        gimp.context_pop()

Then you discover that there are better ways to fill the layer, so you need only one change

Code:

       gimp.context_push()

        # most of your code goes here

        color=[gimp.get_foreground(),gimp.get_background(),background_color][background_type-1]

        gimp.set_foreground(color)

        layer.fill(FILL_FOREGROUND)

        # some more of your code goes here

        gimp.context_pop()

(I tend to use the object methods instead of the pdb.* procedures when they exists, this make the code easier to read. To find what they are, do a dir(gimp), dir(image, or dir(layer) in the python console (after initializing image and layer of course) . The mapping are usually quite obvious.

(*) this is called the DRY principle: "don't repeat yourself". The opposite is of course the WET principle: "We enjoy typing"