root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.63k stars 1.25k forks source link

More convenience when constructing TGraphs #7151

Open hageboeck opened 3 years ago

hageboeck commented 3 years ago

Explain what you would like to see improved

I find this kind of plotting code too cumbersome, it only takes double* and sizes:

  const double x[] = {0, 1, 2};
  const double y[] = {1, 2, 3};
  auto graph = new TGraph(3, x, y);

Optional: share how it could be improved

Provide an overload that constructs from iterable collections and/or std::initializer_list, maybe.

  auto graph = new TGraph({0, 1, 2}, {1, 2, 3);

When the collections don't have the same length, throw std::invalid_argument.

eguiraud commented 3 years ago

Assigning both @couet and @Axel-Naumann because the ticket is about ROOT6 graphics but the improvement might be added to ROOT7 graphics.

couet commented 3 years ago

Nice idea. Except I do not have any idea how to make a such constructor. That should be of course extended to TGraphErrors, TGraphAssymError .etc .. TGraph2D etc .. I am happy to do it .. can you point some example/code I can take inspiration from ?

Axel-Naumann commented 3 years ago

Can't we simply take vector<double>s instead of int size, double*?

couet commented 2 years ago

@hageboeck is Axel suggestion fine for you ?

hageboeck commented 2 years ago

I think that is a good compromise.

Unfortunately, you cannot use the initializer_list syntax as I was hoping: https://godbolt.org/z/4szszTv4v

It works if you call it a bit more explicitly, though:

TGraph gr(std::vector<double>{0., 1.}, {0., 1.});

@Axel-Naumann good enough or can we do better?

Axel-Naumann commented 2 years ago

Instead of modifying TGraph, throw in

   TVectorT(std::initializer_list<Element> &&il);

and it will work.

hageboeck commented 2 years ago

It doesn't work, unfortunately, because overload resolution cannot disambiguate between the TGraph(TVectorD and TGraph(TVectorF cases. Nevertheless, with #9968 merged, one can write:

TGraph gr(TVectorD{1., 2., 3.}, {1., 2., 3.});

It probably won't get much better, so closing this here.

github-actions[bot] commented 2 years ago

Hi @hageboeck, @couet,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely, :robot:

guitargeek commented 4 months ago

Re-opening, since #9968 was not merged after all. This issue should not have been closed before actually merging the PR.

guitargeek commented 4 months ago

Talking again about the proposed interface:

  auto graph = new TGraph({0, 1, 2}, {1, 2, 3);

I mean it looks nice and is probably useful for unit tests, but is it really helping our users?

If you have data that you want to plot, when do you hardcode it in initializer lists instead of reading it from files or getting it programmatically as the output of your analysis?