Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dimension columns aggregation with non-existent values #566

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Area charts with data series started with zero: tooltip fixed.
- Series whose contained ',' and aggregated were not working properly.
- Dimension columns with non-existent values aggregation was undefined.

## [0.12.0] - 2024-07-29

Expand Down
6 changes: 5 additions & 1 deletion src/apps/qutils/canvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <QLinearGradient>
#include <QScreen>

// NOLINTBEGIN(misc-include-cleaner,readability-avoid-nested-conditional-operator)

QColor toQColor(const Gfx::Color &color)
{
return {color.getRedByte(),
Expand Down Expand Up @@ -276,4 +278,6 @@ QFont BaseCanvas::fromGfxFont(const Gfx::Font &newFont, QFont font)
? QFont::StyleOblique
: QFont::StyleNormal);
return font;
}
}

// NOLINTEND(misc-include-cleaner,readability-avoid-nested-conditional-operator)
2 changes: 1 addition & 1 deletion src/base/geom/circle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
#include <cmath>
#include <numbers>
#include <optional>
#include <stdexcept>

#include "base/geom/point.h"
#include "base/math/floating.h"
#include "base/math/tolerance.h"

Expand Down
5 changes: 3 additions & 2 deletions src/base/geom/quadrilateral.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#include "quadrilateral.h"

#include "base/math/tolerance.h"
#include <algorithm>

#include "base/math/floating.h"

#include "point.h"
#include "rect.h"
#include "triangle.h"

namespace Geom
Expand Down
2 changes: 2 additions & 0 deletions src/base/gfx/pathsampler.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#include "pathsampler.h"

#include <cmath>
#include <cstddef>

#include "base/geom/point.h"
#include "base/geom/triangle.h"
#include "base/math/floating.h"

namespace Gfx
{
Expand Down
4 changes: 4 additions & 0 deletions src/chart/rendering/markers/abstractmarker.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#include "abstractmarker.h"

#include <array>
#include <cstddef>
#include <utility>

#include "base/anim/interpolated.h"
#include "base/geom/line.h"
#include "chart/generator/marker.h"
Expand Down
2 changes: 1 addition & 1 deletion src/chart/rendering/renderedchart.cpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#include "renderedchart.h"

#include <algorithm>
#include <limits>
#include <ranges>
#include <variant>

#include "base/geom/point.h"
#include "base/geom/transformedrect.h"
#include "base/math/floating.h"
#include "base/math/fuzzybool.h"
#include "base/util/eventdispatcher.h"
#include "chart/generator/marker.h" // NOLINT(misc-include-cleaner)
#include "chart/options/shapetype.h"
Expand Down
11 changes: 4 additions & 7 deletions src/dataframe/impl/data_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,14 +238,11 @@ cell_reference data_source::get_data(std::size_t record_id,
using enum series_type;
case dimension: {
const auto &dims = unsafe_get<dimension>(series).second;
if (record_id >= dims.values.size()) return nullptr;
return dims.get(record_id);
}
case measure: {
const auto &meas = unsafe_get<measure>(series).second;
if (record_id >= meas.values.size()) return nan;

return meas.values[record_id];
return meas.get(record_id);
}
case unknown:
default: error(error_type::series_not_found, column);
Expand Down Expand Up @@ -627,9 +624,9 @@ void data_source::dimension_t::add_more_data(
const std::string *data_source::dimension_t::get(
std::size_t index) const
{
return values[index] == nav
? nullptr
: std::addressof(categories[values[index]]);
return index < values.size() && values[index] != nav
? std::addressof(categories[values[index]])
: nullptr;
}

void data_source::dimension_t::set(std::size_t index,
Expand Down
5 changes: 0 additions & 5 deletions src/dataframe/interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ struct custom_aggregator
return name <=> oth.name;
}

auto operator!=(const custom_aggregator &oth) const
{
return name != oth.name;
}

auto operator==(const custom_aggregator &oth) const
{
return name == oth.name;
Expand Down
2 changes: 0 additions & 2 deletions src/dataframe/old/datatable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
#include "base/conv/auto_json.h"
#include "base/conv/numtostr.h"
#include "base/refl/auto_enum.h"
#include "base/text/funcstring.h"
#include "chart/options/options.h"
#include "chart/options/shapetype.h"
#include "dataframe/impl/aggregators.h"
#include "dataframe/impl/data_source.h"
#include "dataframe/interface.h"

Expand Down
10 changes: 6 additions & 4 deletions test/qtest/chart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,9 @@ void TestChart::run()
IO::log() << "step 1b";
auto &options = chart.getChart().getOptions();
auto &styles = chart.getChart().getStyles();
options.dataFilter =
Vizzu::Data::Filter{std::shared_ptr<bool(
const Vizzu::Data::RowWrapper *)>{
std::shared_ptr<void>{},
options.dataFilter = Vizzu::Data::Filter{
std::unique_ptr<bool(const Vizzu::Data::RowWrapper *),
void (*)(bool(const Vizzu::Data::RowWrapper *))>{
+[](const Vizzu::Data::RowWrapper *row) -> bool
{
return *std::get<const std::string *>(
Expand All @@ -158,6 +157,9 @@ void TestChart::run()
|| *std::get<const std::string *>(
row->get_value("Cat2"))
== std::string_view{"b"};
},
+[](bool(const Vizzu::Data::RowWrapper *))
{
}}};
options.title = "VIZZU Chart - Phase 1b";
styles.legend.marker.type =
Expand Down
5 changes: 3 additions & 2 deletions test/unit/base/refl/auto_enum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ template <typename T> std::string toString(T v)
{
return Refl::enum_name<std::string>(v);
}
template <typename T> T parse(std::string s)
template <typename T> T parse(const std::string &s)
{
return Refl::get_enum<T>(s);
}
Expand Down Expand Up @@ -120,7 +120,8 @@ const static auto tests =
{
throws<std::logic_error>() << []
{
return Refl::enum_name(Foo::fobar{2});
return Refl::enum_name(
std::bit_cast<Foo::fobar>(2));
};
})

Expand Down
2 changes: 2 additions & 0 deletions test/unit/chart/events.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ struct MyCanvas final : Gfx::ICanvas, Vizzu::Draw::Painter
void frameBegin() final {}
void frameEnd() final {}
void *getPainter() final { return static_cast<Painter *>(this); }
// cppcheck-suppress duplInheritedMember
ICanvas &getCanvas() final { return *this; }
};

Expand Down Expand Up @@ -333,6 +334,7 @@ std::multimap<std::string, event_as, std::less<>> get_events(
"draw-" + std::string{s.back()});
}

// cppcheck-suppress uselessCallsConstructor
if (s.back() == "base") s = {s.begin(), s.end() - 1};

std::string name;
Expand Down
2 changes: 2 additions & 0 deletions test/unit/dataframe/interface_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ struct if_setup

skip->*df->get_dimensions() == dims;
skip->*df->get_measures() == meas;
// cppcheck-suppress ignoredReturnValue
skip->*df->get_record_count() == data.size();

for (auto r = 0u; r < data.size(); ++r) {
Expand All @@ -77,6 +78,7 @@ struct if_setup
std::get<double>(df->get_data(r, meas[m]));
auto &&nData = data[r][m + ds];
if (nData && std::isnan(gdata)) continue;
// cppcheck-suppress ignoredReturnValue
skip->*gdata == std::stod(data[r][m + ds]);
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/unit/util/collection.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class collection : public case_registry
return stats;
}

void print_summary(statistics stats)
void print_summary(const statistics &stats)
{
std::cout << "\n"
<< "all tests: " << cases.size() << "\n"
Expand Down
3 changes: 1 addition & 2 deletions test/unit/util/condition.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ template <typename T> struct decomposer

template <class U> void operator==(const U &ref) const
{
if constexpr (requires { ref == value; }) {
if constexpr (requires { ref == value; })
return evaluate(value == ref, "==", ref);
}
else if constexpr (std::ranges::range<T>) {
auto &&[lhs, rhs] = std::ranges::mismatch(value, ref);
return evaluate(lhs == std::end(value)
Expand Down