[core] Add new ROOT/StringUtils.hxx file with ROOT::Split function by guitargeek · Pull Request #8821 · root-project/root

guitargeek · 2021-08-09T18:06:55Z

The ROOT::Split function splits a string in the same way as the
str.split function from Python.

Closes #8807.

phsft-bot · 2021-08-10T10:20:59Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-08-10T11:05:56Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-10T10:22:36.042Z] stderr: error: could not read '.git/rebase-apply/head-name': No such file or directory

hageboeck

LGTM, but I have 2 more suggestions.

hageboeck · 2021-08-11T13:49:21Z

core/foundation/test/testStringUtils.cxx

+   auto test = [](std::string_view in, std::vector<std::string> const &ref) {
+      auto out = ROOT::Split(in, ",");
+      EXPECT_EQ(out, ref) << "ROOT::Split(\"" << in << "\") gave wrong result.";
+   };
+
+   test("a,b,c", {"a", "b", "c"});
+   test("a,,c", {"a", "", "c"});
+   test("a,,,", {"a", "", "", ""});
+   test(",,a,,,", {"", "", "a", "", "", ""});
+   test(",,,", {"", "", "", ""});
+   test(",,a", {"", "", "a"});
+   test("", {""});


Suggested change

auto test = [](std::string_view in, std::vector<std::string> const &ref) {

auto out = ROOT::Split(in, ",");

EXPECT_EQ(out, ref) << "ROOT::Split(\"" << in << "\") gave wrong result.";

};

test("a,b,c", {"a", "b", "c"});

test("a,,c", {"a", "", "c"});

test("a,,,", {"a", "", "", ""});

test(",,a,,,", {"", "", "a", "", "", ""});

test(",,,", {"", "", "", ""});

test(",,a", {"", "", "a"});

test("", {""});

auto test = [](std::string_view in, std::vector<std::string> const &ref, bool skipEmpty) {

auto out = ROOT::Split(in, ",", skipEmpty);

EXPECT_EQ(out, ref) << "ROOT::Split(\"" << in << "\") gave wrong result.";

};

test("a,b,c", {"a", "b", "c"}, false);

test("a,b,c", {"a", "b", "c"}, true);

test("a,,c", {"a", "", "c"}, false);

test("a,,c", {"a", "c"}, true);

test("a,,,", {"a", "", "", ""}, false);

test(",,a,,,", {"", "", "a", "", "", ""}, false);

test(",,,", {"", "", "", ""}, false);

test(",,,", {}, true);

test(",,a", {"", "", "a"}, false);

test("", {""}, false);

Good idea! I have changed that.

hageboeck · 2021-08-11T13:51:41Z

roofit/roofitcore/src/RooAbsPdf.cxx

 #include "RooHelpers.h"
 #include "RooBatchCompute.h"
 #include "RooFormulaVar.h"
+#include "RooDerivative.h"


That's here intentionally? (One could argue that this better goes into an extra commit, but maybe too much work).

This is just to reorder the includes such that the RooFit includes appear before the remaining ROOT includes. Since I was touching the incudes, I thought why not do that

Sure, that's a good idea, but there was no such include before if I'm not mistaken?
Edit: Ah, never mind. 🙂

The `ROOT::Split` function splits a string in the same way as the `str.split` function from Python.

phsft-bot · 2021-08-11T14:11:01Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-08-11T15:24:01Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-11T14:40:33.757Z] stderr: error: could not read '.git/rebase-apply/head-name': No such file or directory

phsft-bot · 2021-08-11T17:37:03Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

projectroot.roottest.python.function.roottest_python_function_function

guitargeek added in:Core Libraries in:RooFit labels Aug 9, 2021

guitargeek self-assigned this Aug 9, 2021

guitargeek requested review from Axel-Naumann, bellenot, hageboeck, lmoneta and pcanal as code owners August 9, 2021 18:06

guitargeek force-pushed the StringUtils_Split_1 branch from cc46cd1 to b1438e0 Compare August 10, 2021 10:20

root-project deleted a comment from phsft-bot Aug 10, 2021

hageboeck approved these changes Aug 11, 2021

View reviewed changes

guitargeek added 3 commits August 11, 2021 16:10

[core] Add new ROOT/StringUtils.hxx file with ROOT::Split function

7bdce11

The `ROOT::Split` function splits a string in the same way as the `str.split` function from Python.

Use ROOT::Split in hist and tree components

db9ccdd

[RF] Replace RooHelpers::tokenise() with ROOT::Split()

e1efcd3

guitargeek force-pushed the StringUtils_Split_1 branch from b1438e0 to e1efcd3 Compare August 11, 2021 14:10

guitargeek merged commit ed916d2 into root-project:master Aug 11, 2021

guitargeek deleted the StringUtils_Split_1 branch August 11, 2021 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Add new ROOT/StringUtils.hxx file with ROOT::Split function#8821

[core] Add new ROOT/StringUtils.hxx file with ROOT::Split function#8821
guitargeek merged 3 commits intoroot-project:masterfrom
guitargeek:StringUtils_Split_1

guitargeek commented Aug 9, 2021

Uh oh!

phsft-bot commented Aug 10, 2021

Uh oh!

phsft-bot commented Aug 10, 2021

Uh oh!

hageboeck left a comment

Uh oh!

hageboeck Aug 11, 2021

Uh oh!

guitargeek Aug 11, 2021

Uh oh!

hageboeck Aug 11, 2021

Uh oh!

guitargeek Aug 11, 2021

Uh oh!

hageboeck Aug 13, 2021 •

edited

Loading

Uh oh!

phsft-bot commented Aug 11, 2021

Uh oh!

phsft-bot commented Aug 11, 2021

Uh oh!

phsft-bot commented Aug 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-   auto test = [](std::string_view in, std::vector<std::string> const &ref) {
-      auto out = ROOT::Split(in, ",");
-      EXPECT_EQ(out, ref) << "ROOT::Split(\"" << in << "\") gave wrong result.";
-   };
-   test("a,b,c", {"a", "b", "c"});
-   test("a,,c", {"a", "", "c"});
-   test("a,,,", {"a", "", "", ""});
-   test(",,a,,,", {"", "", "a", "", "", ""});
-   test(",,,", {"", "", "", ""});
-   test(",,a", {"", "", "a"});
-   test("", {""});
+   auto test = [](std::string_view in, std::vector<std::string> const &ref, bool skipEmpty) {
+      auto out = ROOT::Split(in, ",", skipEmpty);
+      EXPECT_EQ(out, ref) << "ROOT::Split(\"" << in << "\") gave wrong result.";
+   };
+   test("a,b,c", {"a", "b", "c"}, false);
+   test("a,b,c", {"a", "b", "c"}, true);
+   test("a,,c", {"a", "", "c"}, false);
+   test("a,,c", {"a", "c"}, true);
+   test("a,,,", {"a", "", "", ""}, false);
+   test(",,a,,,", {"", "", "a", "", "", ""}, false);
+   test(",,,", {"", "", "", ""}, false);
+   test(",,,", {}, true);
+   test(",,a", {"", "", "a"}, false);
+   test("", {""}, false);

Conversation

guitargeek commented Aug 9, 2021

Uh oh!

phsft-bot commented Aug 10, 2021

Uh oh!

phsft-bot commented Aug 10, 2021

Errors:

Uh oh!

hageboeck left a comment

Choose a reason for hiding this comment

Uh oh!

hageboeck Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

guitargeek Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

hageboeck Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

guitargeek Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

hageboeck Aug 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phsft-bot commented Aug 11, 2021

Uh oh!

phsft-bot commented Aug 11, 2021

Errors:

Uh oh!

phsft-bot commented Aug 11, 2021

Failing tests:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hageboeck Aug 13, 2021 •

edited

Loading