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

Unicode type iteration #3842

Merged
merged 5 commits into from Mar 14, 2019
Merged

Conversation

stuartarchibald
Copy link
Contributor

This makes the unicode type iterable and also fixes up a selection
of other things that were causing minor issues on the way. Not
limited to:

  • Contractual clarity over reference ownership in iternext_impl
  • Matching debug print with function names in C code
  • PEP8

This makes the unicode type iterable and also fixes up a selection
of other things that were causing minor issues on the way. Not
limited to:

 * Contractual clarity over reference ownership in `iternext_impl`
 * Matching debug print with function names in C code
 * PEP8
@stuartarchibald
Copy link
Contributor Author

Closes #3836 and #3835

@seibert
Copy link
Contributor

seibert commented Mar 8, 2019

Is the reference ownership issue such that it makes sense to be explicit about new_ref=False in all the existing uses of iternext_impl as form of documentation?

@stuartarchibald
Copy link
Contributor Author

RE:#3842 (comment) It could be, wonder if new, borrowed and untracked all ought to be quantified and specification required?

numba/runtime/nrt.c Outdated Show resolved Hide resolved
numba/runtime/nrt.c Outdated Show resolved Hide resolved
numba/runtime/nrt.c Outdated Show resolved Hide resolved
@stuartarchibald
Copy link
Contributor Author

@seibert what do you think of stuartarchibald@83ae051?diff=unified ?

@seibert
Copy link
Contributor

seibert commented Mar 8, 2019

I think that is a good change. Most booleans turn into enums anyway. :)

@stuartarchibald
Copy link
Contributor Author

@seibert thanks, I've appended that patch in 8aa830a

@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 3 - Ready for Review labels Mar 12, 2019
@seibert
Copy link
Contributor

seibert commented Mar 12, 2019

Need to merge master into this PR and fix the dictionary usage of iternext_impl.

@stuartarchibald
Copy link
Contributor Author

This is ready for re-review, dictionaries are fixed up.

@seibert
Copy link
Contributor

seibert commented Mar 14, 2019

Looks good. Ready to merge.

@seibert seibert added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Mar 14, 2019
@sklam sklam added this to Ready to Merge in Active Mar 14, 2019
@seibert seibert merged commit d47ef31 into numba:master Mar 14, 2019
Active automation moved this from Ready to Merge to Done Mar 14, 2019
@ehsantn
Copy link
Collaborator

ehsantn commented Mar 14, 2019

Are the changes to @iternext_impl breaking here? BTW, it is not documented in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants