phatch-dev team mailing list archive

[Juho Vepsäläinen <bebraw@xxxxxxxxx>]

Hi again,

2009/6/14 Stani <spe.stani.be@xxxxxxxxx>:
> 2009/6/14 Juho Vepsäläinen <bebraw@xxxxxxxxx>:
>> Hi,
>>
>> While developing the EXIF related patches I noticed it modifies the
>> metadata of input image(s) in any case (ie. even if "Save" action is
>> present). Is this by purpose? Modifying image data is non-destructive
>> so it kind of contradicts with that. I have not worked with
>> EXIF/metadata related things before so it's all new to me. :)
> Yes this by design. I've explained the principle before with the
> geotag action as that one was the first one manipulating the metadata:
> https://lists.launchpad.net/phatch-dev/msg00102.html
>
> Basically the save action now saves the pil image to disc. In the
> current state it would be not wanted that if you want to add metadata
> (such as geodata or authorship) to a jpeg image that it is opened and
> saved by pil, as this is not a lossless operation. You basically want
> to to open only the metadata and save it. As such in the current
> implementation every metadata action is a save action. However they
> are buffered and written at once. So that is why you can have an
> action list with only a metadata action or why you have to place the
> metadata action *after* the save action, as this won't change the
> source image. If a metadata action after the save action modifies the
> source image it is a bug.
>
> That said,  I agree that, although this design is consequent and
> predictable, it is counter-intuitive in Phatchs workflow. In Phatch
> 0.1 saving metadata was a testing feature and therefore not enabled by
> default. Exiv2 is not perfect and has its own bugs. Therefore I wanted
> to get only bug reports of people who enabled this conciously. For
> Phatch 0.2 I hope we have fixed these issues as good as we can, but
> not completely. (Eg no option to add or update exif thumbnails.) To
> put the "Copy metadata" as a checkbox in the execution dialog box, was
> also for me a temporary solution, as I didn't dare to put it in the
> save action yet. Moreover as there was no system for conditional
> showing/hiding options the save list would become too long.
>
> So now we have dealt with most exiv2 issues (which means even
> disabling where exiv2 fails, eg for tuple values) and have conditional
> parameters at our disposal, we can design a better way how to
> integrate the metadata workflow in Phatch. There a different solution
> possible:
> - we could add image_dirty or pyexiv2_dirty attribute to actions.
> (Dirty means changed.) Whatever which is dirty is saved by the save
> action. Disadvantage here is that the save options such as quality and
> so on ... are not taken in consideration when only the metadata is
> dirty. So if you want to save with manipulated metadata and another
> compression there is no way to do so.
> - a proposal could be to add a ChoiceField to the save action which
> let's you choose between saving image, metadata or both. This has the
> advantage that the user is in full control and that optionally the
> right parameters can be shown or hidden.
> - ...
> I'm curious to hear your proposals, as there should be more.

Would it make sense to separate the metadata part from image editing?
Then possibly different kind of workflow could be justified and
documented better. So perhaps the application would have separate tabs
for editing images and metadata. It probably boils down to how the
application is actually used. :)

>>
>> Is there some specific place where I should put my tests? Do you
>> prefer to use some specific testing suite (ie. unittest/doctest/some
>> other?)?
> Look to core/lib/formField.py I started implementing doctests there
> already as an example. However I had not yet the time to finish it
> yet. So always use doctests, unless it doesn't make sense. The
> unittests will be run by nose.

Alright. Great.

>>
>> I have used "get_relevant_field_labels" for conditional fields (ie.
>> fields depending on state of others). The system works, yes. But I
>> find it a bit suboptimal.
> Suboptimal in which way? Can you clarify?

As stated below it felt as if "get_relevant_field_labels" was a bit
too elaborate whereas more compact representation might have sufficed.
I can see the justifications of the system better now.

>> Is there some specific reason why not to
>> determine the relationships between different fields in "interface"? I
>> think it might be more straightforward this way.
> This is about relations in between fields, so I am curious how you can
> define these inside the fields themselves. Please give an example. The
> only thing I know of are xforms (from where I took the relevance
> concept), but it would be overkill to work with xforms here. Also
> having it defined in one python function gives you a lot of
> flexibility without being complex.
> Actually I am quite happy with this method. But in case you have
> something better in your mind please propose an alternative so we can
> compare and evaluate.

The concept below is meant to ~support~, not totally replace the
current system. I can whip up a proof of concept to show how it would
fit the system.

>>
>> In addition it might be interesting to construct the object hierarchy
>> based on higher level definition (ie. YAML). Here's an example (ported
>> "reflection" action to YAML syntax):
>> ui = '''
>> PixelField:
>>    -name: Depth
>>    -width: 50% # I suppose original 50% meant width. anyway as you
>> can see it's reader friendly too
>>    -choices: self.PIXELS[:-1]
>> PixelField:
>>    -name: Gap
>>    -width: 0
>>    -choices: ['0', '1', '2', '5']
>> SliderField:
>>    -name: Opacity
>>    -value: 60
>>    -min: 0
>>    -max: 100
>> BooleanField:
>>    -name: Scale Reflection
>>    -value: False
>>    -children:
>>        ImageResampleField:
>>            -name: Scale Method
>>            -method: antialias
>> ColorField:
>>    -name: Background Color
>>    -color: black # I prefer to use a library
>> (http://code.google.com/p/grapefruit/) handling the conversion
>> SliderField:
>>    -name: Background Opacity
>>    -value: 100
>>    -min: 0
>>    -max: 100
>>
>> I'm not 100% sure that it's valid YAML. It should be right enough to
>> convey the idea though. Basically keys refer to class names and their
>> attributes to the attributes of the classes. It's a piece of cake to
>> convert this into a real object hierarchy if needed.
> Hmmm... how this system deals with gettext with extracting
> information? One of the rules I used in Phatch development to use
> python for everything: so no xml, ... Actionlists are dictionaries and
> the settings are even pickled dictionaries. As Phatch started as one
> mans project this has saved a lot of time and complexity. If more
> people are willing to put their time in these issues, we can revaluate
> that and maybe get to something better.

I think it's quite possible to embed Python within YAML definition.
Even better would be to have names ran through translator function
separately. It has been a while since I had to work with gettext but I
don't believe it will become too big a problem (it's my problem anyway
:) ).

> Until now I have no regrets. I really like that actions are one file
> as it makes it easy to handle. From the moment you start with multiple
> files (like an xml/yaml for user interface, pil function in py file
> and icon seperately), you need to manage. Do you put each action in a
> separate folder or zip or ...? So I took the KISS approach.

Note that the UI definition can be included in the .py file itself.
Basically it would be just a string assigned to a class attribute. I
don't see much sense in separating it from the .py file. Note that
YAML is more about communicating structure whereas XML is more fitting
for purposes where you need to use a schema (ie. metadata to describe
structure of data). I rather would not use XML in this case.

In the case of icons I might be inclined to favor separate files and
then refer to them just by using a name. Perhaps the arch should
support user defined icons even? One interesting aspect of open source
development is that if you provide the users affordance (ie. empower
them!), they might contribute something back in the form of enhanced
icons/whatnot. By the way I'm not saying that the current icons are
bad. On the contrary. :)

> So I am open for new systems if it can do everything as the current
> system and better. In fact there are some parts where I consciously
> had to take a shortcut and I know there are better solutions. For
> example in action lists the english label fields are used as an ID.
> This is really bad as this means that if for example you correct a
> spelling mistake in a label it breaks a previous saved action list. So
> there is still a lot of place for improvement.

Yeah. I fought with the name/ID issue as well. :) There are at least
two ways how to handle this issue:
1. provide optional id parameter to Field (ie. fields[_t('Value')] =
self.CharField('Phatch', id='value'). Then you can do value   =
self.get_field(id='value',info). If possible I would try to eliminate
the info parameter too.
2. provide optional id parameter as in 1. and get value from self like
this: self.fields.value . So basically the system generates the wanted
attributes based on ids.

2. feels more natural to me. Of course the problem is that it pretty
much forces you to invent id should you want to refer to a field later
but is that such a bad thing? Besides explicit ids it might be
possible to use some rule (ie. order of fields) but I yet have to
figure out a reliable and nice way to do this.

>>
>> Note that everything needed by interface and get_relevant_field_labels
>> methods can be generated based on this. So if wanted, this system can
>> fit on top of the current one instead of replacing it. I think the
>> system would pretty much pay off for itself in more complex cases (ie.
>> hierarchies going past two level). Furthermore it could be used to
>> bring cohesion and consistency to the system.
> I'm afraid we need more power for the relevancy. (Check out Xforms.
> Inkscape is going to use that.) For example for the varborder action,
> I wanted to implement the opacity is only shown if the values for
> left, top, bottom and right are negative. How would you do that with
> YAML? I like a relevance method as it is dynamic and very flexible,
> while YAML looks static. The current proposal looks 'suboptimal' to
> me, but prove me wrong ;-)
>

Okay. That's a good question. :) For me YAML is just pure, initial
structure of the whole. Basically you get nice, ~visual~
representation of the structure.

To determine extra logic you would have to attach a handler to element
and then check logic there. So basically that would be one extra line
to YAML definition (handler: my_awesome_opacity_toggler) and
implementation of the func in action. Obviously each of these handlers
would have to be invoked every time a value is altered (more
sophisticated system would be able to minimize the amount of calls but
even this would do the trick for time being).

So just to make it clear, YAML is just a starting point. You can do
all kind of fancy stuff with handlers hooked up with it. It should
scale more than well to the basic cases (most of actions are pretty
simple) and then some.

>>
>> I suppose many actions have some sort of background color and opacity
>> defined in them. It would be possible to refer to generic properties
>> such as this in the structure itself. In this case you would subclass
>> ColorField and SliderField and end up with something like this:
>> BackgroundColor
>> BackgroundOpacity
>>
>> Of course sometimes you might want to this partially (ie. Opacity):
>> Opacity: # subclass of SliderField
>>    -name: just some opacity
>>    -value: 55 # i'm not happy with the default so let's set it to 55 :)
>>
>> Anyway those were some initial thoughts after two days of development.
>> It has been pretty easy to work with phatch so far. :)
> Well many features are implemented in a few hours or less, so you see:
> two days of development is already plenty enough to give feedback.
> Just take in account that besides features, the transition from Phatch
> 0.1 to 0.2 is also from an one person project to a multiple persons
> project. I feel still too much load on my shoulders, but with so much
> energy I hope that can diminish ;-)
>
> Probably for these kind discussions IRC is a better medium (#phatch on
> freenode). I'll be there monday.
>

Okay. From my point of view as a scientist it would be interesting to
understand how the transition from one to many (cathedral to bazaar)
happens. More specifically what triggers it. Perhaps you can share
your experiences later. :)

I hope my initial comments were not too harsh. I still have some in
store (ie. proper preview and co.). :)

Sincerely,
Juho Vepsäläinen

> Best regards,
>
> Stani
>
> --
> Phatch Photo Batch Processor - http://photobatch.stani.be
> SPE Python IDE - http://pythonide.stani.be
>