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

Add accessor for range #1397

Closed
papadop opened this issue Feb 11, 2019 · 3 comments
Closed

Add accessor for range #1397

papadop opened this issue Feb 11, 2019 · 3 comments

Comments

@papadop
Copy link
Contributor

papadop commented Feb 11, 2019

This is my first try at really using xtensor... so please excuse me if I missed something.

In my use case, I have to create a block matrix, so I want to create a list of contiguous ranges, each new range starting at the stop value of the previous one. Right now I cannot use the xt::range (actually xrange_adaptor) facility as it is impossible to access to the range components.
So the question is would it be acceptable to give const access to the range components (start, stop, and step) so that range can be used in algorithms. Otherwise, I need to create my own range class which will be redundant....

It's just a matter of adding the const accessors to the private fields (I can submit a patch if the idea is accepted). This would also be helpful for debugging purposes as the user can then print the computed ranges...

By the way, I do not understand why the methods normalize and get_stepped_range are not static and private. Static potentially slightly improves the efficiency (not having to pass this and the compiler may use the fact that it does not depend on the object members).

@JohanMabille
Copy link
Member

Hi,

No problem for adding accessors to the fields. Regarding the normalize and get_stepped_range, this sounds as an acceptable change (although I'm not sure that this can be a bottleneck).

@papadop
Copy link
Contributor Author

papadop commented Feb 12, 2019

Thank's for the answer.
See PR #1399.

@SylvainCorlay
Copy link
Member

Fixed in #1401

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

3 participants