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

Bug fix for deque and vector random-access iterators #1870

Merged
merged 1 commit into from Oct 24, 2017

Conversation

Alexhuszagh
Copy link
Contributor

As mentioned in issue #1867:

  1. Added deque support for random-access iterators
  2. Changed various reverse_iterator methods to return reverse_iterator rather than iterator.
  3. Added difference_type operator-(iterator) for all random-access iterators.

@scoder
Copy link
Contributor

scoder commented Sep 13, 2017

The libcpp_all test (which just cimports what's there) now fails:

9:12: 'difference_type' is not a type identifier
22:12: 'difference_type' is not a type identifier

It would be good to have at least one additional test for each of the classes that you touched, to make sure that these declarations actually work when used. Look through the cpp_*.pyx files in tests/run/, there should be some where you can add this. The are driven by Python code in doctests, but the test files themselves are compiled.

@Alexhuszagh
Copy link
Contributor Author

Let me amend my git commit history to fix these issues and then add a separate commit with the tests. I'm missing a ctypedef, so this should be trivial.

…e_type operator-(iterator)" for all random-access forward and reverse iterators
@Alexhuszagh
Copy link
Contributor Author

@scoder, I've updated the patch to include the tests you mentioned, and fix the bugs with the original patch. The new version now ensures operator-() and operator+() functionality for both vector and deque iterators.

@Alexhuszagh
Copy link
Contributor Author

@scoder Any other issues with my PR? Sorry for the massive delay on my end.

@scoder
Copy link
Contributor

scoder commented Oct 24, 2017

Sorry, must have missed your message. Looks good to me, thanks!

@scoder scoder merged commit 62f776c into cython:master Oct 24, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants