-
Notifications
You must be signed in to change notification settings - Fork 337
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
Comments
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 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 = {}); |
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. |
I totally subscribe to the request of making matplot++ work with std ranges. Vector is simply too restrictive. Something like
should work. Moreover, this change should go beyond simply accepting std::range inputs only to convert them to std::vector, as e.g. in axes_type::plot(). The caller can easily convert the ranges to vectors prior to plot(), but that defeats the purpose. The point is to avoid the copy to the internal storage x_data_ and y_data_. This would require that x_data_, y_data_ and z_data_ be ranges rather than vectors. I've skimmed through line.cpp to see how line::x_data is being used and for the most part it uses only the ranges intefrace (x_data.begin(), etc.) except in line::data_string(), where it is being index into:
Similarly for y_data. I haven't looked into the call to parent->draw_path() in list::run_draw_commands(). This would be a rather comprehensive change, but I'm willing to help if you think it's worthwhile. |
I cannot implement it, but I agree with the proposal. The plot types would ideally hold a type-erased view of the range instead of storing vectors. This would probably involve some small private implementation of |
Bug category
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 becauseIterableValues
requires definitions ofconst_iterator
type,begin/end
andsize
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.
It is due to the failure of the template arguments T3 and T4 deduction when calling the
scatter
member function ofaxes_type
below.So the
scatter
function needs explicit 4 arguments to use ranges other thanstd::vector
, likescatter(x, y, std::list<double>{}, std::list<double>{})
.Platform
The text was updated successfully, but these errors were encountered: