Algorithms to find values and indices in vectors of data
AcceptedPublic

Authored by Murmele on Jan 16 2019, 12:52 PM.

Details

Reviewers
asemke

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Murmele created this revision.Jan 16 2019, 12:52 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptJan 16 2019, 12:52 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
Murmele requested review of this revision.Jan 16 2019, 12:52 PM

Thanks in advance for fixing minor Doxygen issues.

src/backend/worksheet/plots/cartesian/XYCurve.cpp
1721

Here, in this line, that should be @p x and @p valueFoundto avoid Doxygen error.

1751

@p x

1883

@p x in a vector of values

1957

@p x

2031

@p x

Murmele updated this revision to Diff 49652.Jan 16 2019, 4:22 PM
Murmele marked 5 inline comments as done.
asemke added inline comments.Jan 16 2019, 9:06 PM
src/backend/worksheet/plots/cartesian/XYCurve.cpp
1690

should we define this static?

1744

In indexForX() you have handling for datetime. Why not to add this here, too?

1766

this three lines can be simplified to

bool increase = (properties != AbstractColumn::Properties::MonotonicDecreasing);
1773

maxSteps, camel case...

src/backend/worksheet/plots/cartesian/XYCurve.h
72

indexForX oder indexForXinColumn? Let's maybe name all these functions simply indexForX and diferentiation is done by the parameters and the documentation of these overloaded parameters. This would be similar for example to the all those map*() functions in CartesianCoordinateSystem.

Murmele added inline comments.Jan 17 2019, 7:27 AM
src/backend/worksheet/plots/cartesian/XYCurve.cpp
1744

should there datetimes on the y axis?

Murmele marked 4 inline comments as done.Jan 17 2019, 7:30 AM
Murmele updated this revision to Diff 49689.Jan 17 2019, 7:35 AM
  • renamed functions
  • renamed max_steps to maxSteps
  • made calculateMaxSteps to camel case
  • simplified some code
Murmele updated this revision to Diff 49690.Jan 17 2019, 7:38 AM
Murmele updated this revision to Diff 49692.Jan 17 2019, 7:50 AM

style fixes

Murmele updated this revision to Diff 49753.Jan 17 2019, 8:59 PM

Add function for finding QDateTime

Murmele updated this revision to Diff 49755.Jan 17 2019, 9:13 PM
asemke accepted this revision.Jan 17 2019, 9:15 PM
This revision is now accepted and ready to land.Jan 17 2019, 9:15 PM

Causes build failures on KDE Neon builds.

https://build.neon.kde.org/job/bionic_unstable_extras_labplot/188/

00:40:09 /workspace/build/src/backend/worksheet/plots/cartesian/XYCurve.cpp: In static member function ‘static int XYCurve::calculateMaxSteps(unsigned int)’:
00:40:09 /workspace/build/src/backend/worksheet/plots/cartesian/XYCurve.cpp:1708:2: error: narrowing conversion of ‘-1’ from ‘int’ to ‘char’ inside { } [-Wnarrowing]
00:40:09 };
00:40:09 ^

Causes build failures on KDE Neon builds.

https://build.neon.kde.org/job/bionic_unstable_extras_labplot/188/

00:40:09 /workspace/build/src/backend/worksheet/plots/cartesian/XYCurve.cpp: In static member function ‘static int XYCurve::calculateMaxSteps(unsigned int)’:
00:40:09 /workspace/build/src/backend/worksheet/plots/cartesian/XYCurve.cpp:1708:2: error: narrowing conversion of ‘-1’ from ‘int’ to ‘char’ inside { } [-Wnarrowing]
00:40:09 };
00:40:09 ^

Does it solve to explicitly set: "const char LogTable256[256]" to "const signed char LogTable256[256]" ?

Yes. Whether char is signed or unsigned is implementation-dependent. Using "signed char" should fix this.

I included calculateMaxSteps() as nsl_sf_basic_log2p1_int() in NSL. Please use this function now.
I will add performance and accuracy tests for different implementations soon.