This is the start of an implementation of positioned stops for GradientFill. GradientFill.stop may now be specified as a list of:
(color, position) pairs
However, I can't seem to get all of the Serializable magic working; the default to_tree doesn't seem to be giving me what I expect. (Getting <stop val="<openpyxl.styles.fills.Stop object> ....) Please help, @CharlieC!
(PS: I'm failing to correctly use hg branches, with apologies)
What problems are you having with hg branches? Looks okay at the moment. Important is not to be working on the default branch. I think this is where openpyxl differs from most projects but we have a reason for not making a default checkout bleeding edge.
Could you please comment on why the serialization is not working? TestGradientFill.test_serialize fails with
E <gradientFill bottom="4" degree="90" left="1" right="2" top="3" type="linear">
E <stop val="<openpyxl.styles.fills.Stop object>
E position=0.0, color=<openpyxl.styles.colors.Color object>
I have no idea what real world examples for the 'path' type are except perhaps to create some kind of border...?
The case I was considering for 'linear' was using this as a bar chart, i.e. [BLUE, (RED, .7), (WHITE, .7), WHITE] for a value of 70%. I don't understand how this helps as a unit test, though, rather an end-to-end test.
I admit I do not know other contexts where a multi-part gradient is useful in excel.
Borders can be created with the edge attributes: the specification contains an example. Your suggested example is fine but I would suggest that the code initially be fill.stop = [Stop(BLUE, 0), Stop(RED, ,7), Stop(WHITE, .7), Stop(WHITE, 1)] I'm guessing at the positions but this will help us get the API (the way we want client code to work) right.
Although I've just tested my Excel for Mac installation and found that two identical positions does not work. So you need to do (0, .7, .7001, 1). My Excel also allows stops to be specified in any order (which is one reason why they must be distinct), and it will do the sort.
[MS-OI29500: Microsoft Office Implementation Information for ISO/IEC-29500 Standard Compliance]18.8.38. stop (Gradient Stop)(a)The standard specifies @position as defined by the XML Schema double datatype.Office requires @position to be a double between 0 and 1 inclusive.
It's can be difficult to enforce constraints as this level so I wouldn't worry about them too much. It might be easier to have some kind of "gradient factory" that client code can use to get values right. But we'll cross that bridge when we come to it.
Charlie, i understand how to make this work and provide the interface you
consider appropriate, i think. I have not understood how to make it
backwards compatible, automatically enumerating positions where none are
supplied, if not in the descriptor. Do you not care for such things?
As I've repeatedly noted, backwards compatibility, can and should be done after the code implements the spec correctly and the documentation is improved. If necessary I can do this myself. But I do also think that the desire to support (BLACK, (WHITE, 0.7), RED, BLUE (0.4)) is probably too ambitious and the Stop definition must be robust enough to reject invalid XML. Client code can either provide proper Stops or a list of colours. This makes everything a lot clearer. The code that calculates stop positions is a utility that client code can call with potentially incomplete definitions though I'm not sure if it's actually necessary. But then I don't understand the position stuff very well. I will also submit a bug report to the ECMA WG regarding the constraints
Okay. I'm not sure when I'll get to this, but I intend to (or someone else can take up the baton):
provide you with a sample rendering of a few examples of linear and path gradients with 1, 2, 3 stops, following the spec, and a few examples that fail to render in my Office instance, upon which tests will be based
make the GradientFill.stop setter accept either a list of 1 or 2 colours, or a list of Stops of arbitrary length
we may later extend GradientFill.stop's setter to support (colour, position) pairs (as this is how I would prefer to describe a gradient), or use a convenience openpyxl.styles.fills.make_gradient_stops() helper
we may later extend handling of multiple unpositioned colours to support more than two colours with equal lengths between, e.g. for five colours (0, .25, .5, .75, 1) (but you will have to allow tests to account for numerical imprecision)
we could outlaw duplicate positions either in the GradientFill.stop setter or in its to_tree or we could not handle this case and just let the user deal with it
There I manually entered some variants on gradientFill. I got read errors upon opening and still haven't managed to diagnose what is producing the error. But the only cell that didn't render is that with stop position > 1.
Thanks for the example. Difficult to tell why Excel doesn't like the file: the validator flags up all the attributes as being wrong but I guess it's really the ones where position > 1 that cause problems. The implementer notes mention the constraint but I suspect that there should also be a remark noting that stops and positional attributes are mutually exclusive.
I will create bug report for the OOXML to get the spec updated.
by default in path, the gradients end in the centre. if left, etc are
specified, they are compressed into the specified proportion in each
direction, and maintain the final colour. I think that's what you're seeing
Okay. I guess it doesn't really matter too much as these fills, as we can see, are not universally supported. But useful for the documentation. The important thing for the library is that they are correctly preserved in existing files.
Looks pretty good. The logic for dealing with a list of colours is more or less there in the existing _serialise_stop() function. This can be modified to accept a sequence of Stops or Colours and calculate the position as necessary. It could then be used by the __set__ method of the descriptor. fill_unspecified_stops() looks superfluous now, right?
It looks okay but difficult to test. As most of the code touches neither the descriptor (self) nor the GradientFill (obj), it would be better as a (private?) function. This would make testing a lot easier.