XDDF implementation shared between XSSFChart and XSLFChart

[Alain FAGOT BÉAREZ <AB...@FOR-Scala.IT>]

Dear all,

Since June, this is the third pull request I open about the same theme.

After the first review, mentioning a conversation I had missed (https://lists.apache.org/thread.html/ff5470bc413d298188f8c5d3d250ee15d36bd655227696869ed21aab@%3Cdev.poi.apache.org%3E), I reworked my initial submission to refactor my code into some sharable XDDF package for the common DrawingML subset about charts.

After I had opened the second pull request, I wanted to solve some personal situation which my initial XDDF implementation did not allow me to get done, not even with ugly code. So I decided to go for some more steps, learning from the XSSF charts implementation, and migrating the existing XSSF charts and tests to use the new XDDF code, which evolved a lot.

This is why we now have pull request #68 (https://github.com/apache/poi/pull/68 <https://github.com/apache/poi/pull/68>) and pull requests #61 and #67 have been closed.

Personally, I was scratching my own itch which was bug #57835 (https://bz.apache.org/bugzilla/show_bug.cgi?id=57835), at least the Charts part of the bug report. Maybe the Notes part could be solved following some strategy similar to what I implemented for Charts. It did not enter my scope yet. Anyway, it would be worth its own pull request, since this one is already touching 97 files…

I hope this contribution does not fall in the middle of some important release preparation.

Best regards,
Alain

Re: XDDF implementation shared between XSSFChart and XSLFChart

[Greg Woolsey <gr...@gmail.com>]

I'm all for standardizing the API between modules, however I have a large
amount of code using the current XSSF API, in conjunction with another
library, Vaadin-Spreadsheet.  I'm in the process of submitting a large
change request to that group , adding my features to their product.  They
already are slow to adopt POI updates when APIs have breaking changes.  I'm
worried a large change to 3.17 will disrupt my work there, now that I've
finally got their attention.

I would prefer this wait until 3.18, for purely selfish reasons, as we've
already released a beta for 3.17.

Greg

On Fri, Aug 11, 2017, 03:14 kiwiwings <ki...@apache.org> wrote:

> Hi Alain,
>
> I have this issue on my todo-/watchlist, but I currently haven't got much
> time for POI.
> So if no-one else jumps in, you can ping me.
>
> We haven't yet started with the preparations for 3.17 final/4.0 - so I
> think, we'll get that one in before.
>
> Best wishes,
> Andi
>
>
>
> --
> View this message in context:
> http://apache-poi.1045710.n5.nabble.com/XDDF-implementation-shared-between-XSSFChart-and-XSLFChart-tp5728473p5728474.html
> Sent from the POI - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>

Re: XDDF implementation shared between XSSFChart and XSLFChart

[kiwiwings <ki...@apache.org>]

Hi Alain,

I have this issue on my todo-/watchlist, but I currently haven't got much
time for POI.
So if no-one else jumps in, you can ping me.

We haven't yet started with the preparations for 3.17 final/4.0 - so I
think, we'll get that one in before.

Best wishes,
Andi



--
View this message in context: http://apache-poi.1045710.n5.nabble.com/XDDF-implementation-shared-between-XSSFChart-and-XSLFChart-tp5728473p5728474.html
Sent from the POI - Dev mailing list archive at Nabble.com.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

Re: XDDF implementation shared between XSSFChart and XSLFChart

[Greg Woolsey <gr...@gmail.com>]

Vaadin7 requires Java 7, Vaadin 8 requires Java 8, so they are OK there.
POI 3.17 already has breaking changes, so they have to put off
incorporating my work until a larger release anyway, due to possible
customer dependencies.  Their next release will at least be using 3.16, so
users like me can drop in a POI version up through 3.17 before more
referenced deprecated APIs are removed.

As I diagram it more on my end, and work through the details, I suppose
putting this in 3.17 might be fine on all fronts, as long as the changes
follow the POI guidelines of deprecating but not removing methods and
classes.  It just means a fair amount of rework for me on my dependent
projects and contributions, whenever it happens.  There is a lot of code
around OOXML charts in my project and Vaadin's.

I suppose that means others will have the same issue - if we radically
change things up, it will break a lot of code for a lot of people.
Designing and planning such a move should be done carefully, APIs are about
the hardest thing to do well, and require a lot of up-front design work,
I've found.

I'll start by finding some free time to look at the pull request, and see
how much it breaks things for me.

On Fri, Aug 11, 2017 at 3:03 PM Andreas Beeker <ki...@apache.org> wrote:

> > I would prefer this wait until 3.18, for purely selfish reasons, as we've
> > already released a beta for 3.17.
>
> The postponing is ok for me ... but afterwards you have the breaking
> changes anyways
> and the Vaadin guys (or you?) have to do the chart modifications twice ...
> or Vaadin
> might be stuck on 3.17 ...
>
> On the other hand ... I don't know to what degree POI is part of Vaadin,
> but POI 4.0 would
> lift the Java minimum [1]
>
> Andi
>
> [1]
> https://vaadin.com/vaadin-documentation-portlet/framework/installing/installing-java.html
>
>

Re: XDDF implementation shared between XSSFChart and XSLFChart

[Andreas Beeker <ki...@apache.org>]

> I would prefer this wait until 3.18, for purely selfish reasons, as we've
> already released a beta for 3.17.

The postponing is ok for me ... but afterwards you have the breaking changes anyways
and the Vaadin guys (or you?) have to do the chart modifications twice ... or Vaadin
might be stuck on 3.17 ...

On the other hand ... I don't know to what degree POI is part of Vaadin, but POI 4.0 would
lift the Java minimum [1]

Andi

[1] https://vaadin.com/vaadin-documentation-portlet/framework/installing/installing-java.html

Re: XDDF implementation shared between XSSFChart and XSLFChart

[Greg Woolsey <gr...@gmail.com>]

That might work for me, I'm not sure.  Here's more background.

There is much I want to do with the charts API already, but I haven't had
time to see if this change goes in directions I'm wanting already.  In
particular I am constantly having to do variations on "instanceof" and type
casting and/or use reflection to retrieve elements common to most or all
chart types but defined without a common interface or type hierarchy.  The
current design follows the OOXML definitions, which have much overlap and
duplication but don't lend themselves to identifying common attributes.
I've started doing some of that with a parent class for axes, but there
needs to be more.  This XDDF work could help that, and at worst would
likely only move the work to a different package, not add to it.

Delegating the public API, however, may break the necessary introspection
and accessing of CT* classes done currently by both my own project and
Vaadin-Spreadsheet-Charts.  That would be a serious problem, as I'm having
to walk a very fine line already with them - they don't dedicate a lot of
resources, and I'm having to do all the heavy lifting, but I'm a one-person
operation, in charge of all of development and IT for my employer.  The
project using POI is only supposed to be half my time :)

Without testing it, I don't know yet how extensive the changes would be and
how much backward compatibility would be broken, especially given that some
of that compatibility uses reflection to access fields not necessarily
already made public, because not all chart features are accessible from the
POI API.

On Fri, Aug 11, 2017 at 9:16 AM Javen O'Neal <on...@apache.org> wrote:

> That sounds like a good compromise.
>
> On Aug 11, 2017 04:18, "pj.fanning" <fa...@yahoo.com> wrote:
>
> > Would it be feasible to keep the existing classes and APIs and have them
> > delegate to the XDDF impl?
> > We could immediately deprecate any of these legacy APIs.
> >
> >
> >
> > --
> > View this message in context: http://apache-poi.1045710.n5.
> > nabble.com/XDDF-implementation-shared-between-XSSFChart-and-XSLFChart-
> > tp5728473p5728476.html
> > Sent from the POI - Dev mailing list archive at Nabble.com.
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> > For additional commands, e-mail: dev-help@poi.apache.org
> >
> >
>

Re: XDDF implementation shared between XSSFChart and XSLFChart

[Javen O'Neal <on...@apache.org>]

That sounds like a good compromise.

On Aug 11, 2017 04:18, "pj.fanning" <fa...@yahoo.com> wrote:

> Would it be feasible to keep the existing classes and APIs and have them
> delegate to the XDDF impl?
> We could immediately deprecate any of these legacy APIs.
>
>
>
> --
> View this message in context: http://apache-poi.1045710.n5.
> nabble.com/XDDF-implementation-shared-between-XSSFChart-and-XSLFChart-
> tp5728473p5728476.html
> Sent from the POI - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>

Re: XDDF implementation shared between XSSFChart and XSLFChart

["pj.fanning" <fa...@yahoo.com>]

Would it be feasible to keep the existing classes and APIs and have them
delegate to the XDDF impl?
We could immediately deprecate any of these legacy APIs.



--
View this message in context: http://apache-poi.1045710.n5.nabble.com/XDDF-implementation-shared-between-XSSFChart-and-XSLFChart-tp5728473p5728476.html
Sent from the POI - Dev mailing list archive at Nabble.com.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org