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

Support for ANY ranges? #382

Open
2 of 8 tasks
thayakawa-gh opened this issue Dec 2, 2023 · 2 comments
Open
2 of 8 tasks

Support for ANY ranges? #382

thayakawa-gh opened this issue Dec 2, 2023 · 2 comments

Comments

@thayakawa-gh
Copy link

Bug category

  • bug - compilation error
  • bug - compilation warning
  • bug - runtime error
  • bug - runtime warning
  • bug - logic error

Describe the bug
Thank you for your work on this great software, but I'm facing some difficulties with it. I would appreciate if you could help me.

The document says that matplot++ can handle any ranges, but it seems not to be true (e.g., <ranges> in C++20, C-style arrays). This is because IterableValues requires definitions of const_iterator type, begin/end and size member functions. These are different from the general concept of range in C++ and so neither <ranges> nor C-style array satisfy them.
I would like to suggest modifying them to the same requirements as the range-based for loop (since C++17).

In addition, the following code cannot be compiled with Visual Studio 2022.

std::list<double> x{ 1, 2, 3, 4, 5 }, y{ 1, 2, 3, 4, 5 };
scatter(x, y);

It is due to the failure of the template arguments T3 and T4 deduction when calling the scatter member function of axes_type below.

template <class T1, class T2, class T3, class T4>
line_handle scatter(const IterableValues<T1> &x,
                    const IterableValues<T2> &y,
                    const IterableValues<T3> &sizes = {},
                    const IterableValues<T4> &colors = {}) {
    return scatter(to_vector_1d(x), to_vector_1d(y),
                   to_vector_1d(sizes), to_vector_1d(colors));
}

So the scatter function needs explicit 4 arguments to use ranges other than std::vector, like scatter(x, y, std::list<double>{}, std::list<double>{}).

Platform

  • cross-platform issue - linux
  • cross-platform issue - windows
  • cross-platform issue - macos
@alandefreitas
Copy link
Owner

Yes. The documentation exposition was written before "ranges" were as formal as they are today. Even proper C++17 compilers were hard to find at the time.

We need to update IterableValues and the proper definition would need to either explicitly define the overloads without the default parameters because there's no way to infer T3-T4:

template <class T1, class T2>
line_handle scatter(const IterableValues<T1> &x,
                    const IterableValues<T2> &y);

template <class T1, class T2, class T3>
line_handle scatter(const IterableValues<T1> &x,
                    const IterableValues<T2> &y,
                    const IterableValues<T3> &sizes);

template <class T1, class T2, class T3, class T4>
line_handle scatter(const IterableValues<T1> &x,
                    const IterableValues<T2> &y,
                    const IterableValues<T3> &sizes,
                    const IterableValues<T4> &colors);

or just setting default templates might work depending on how IterableValues is defined (I can't remember exactly of the caveats):

template <class T1, class T2, class T3 = T1, class T4 = T1>
line_handle scatter(const IterableValues<T1> &x,
                    const IterableValues<T2> &y,
                    const IterableValues<T3> &sizes = {},
                    const IterableValues<T4> &colors = {});

@thayakawa-gh
Copy link
Author

Thank you for your reply.

I agree with your modification using the simple overloads. I'm not aware of the detailed rules of the template deduction, but it seems that default template arguments, as in the latter case, cause a compilation error in clang.

I'm considering introducing Matplot++ to my data analysis library for visualization, which is based on the range/view concepts in C++20. It would be very helpful for me if you could modify them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants