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

Fix example and docs typos for objmode context manager. #3577

Merged
merged 2 commits into from Dec 12, 2018

Conversation

stuartarchibald
Copy link
Contributor

As title.

Fixes #3575

@codecov-io
Copy link

Codecov Report

Merging #3577 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3577   +/-   ##
=======================================
  Coverage   80.69%   80.69%           
=======================================
  Files         393      393           
  Lines       80425    80425           
  Branches     9160     9160           
=======================================
  Hits        64896    64896           
  Misses      14109    14109           
  Partials     1420     1420


@njit
def foo():
x = np.arange(5)
y = np.arange(5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change it so that we allocate y = np.zeros_like(x) and leave x as is. We want to show that input arrays do not need to be annotated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, thanks, fixed in eacd307

with objmode(y='intp[:]'): # annotate return type
# this region is executed by object-mode.
y += bar(x)
y += bar(y)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't have to change this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in eacd307

@stuartarchibald
Copy link
Contributor Author

Thanks for the review. I also note that numba.cuda.tests.cudapy.test_atomics.TestCudaAtomics failed a test, which has nothing to do with this change.

Implement @sklam's suggestions.
@seibert seibert added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Dec 11, 2018
@seibert seibert added this to Ready to Merge in Active Dec 11, 2018
@stuartarchibald stuartarchibald merged commit ff74d7f into numba:master Dec 12, 2018
Active automation moved this from Ready to Merge to Done Dec 12, 2018
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 doc
Projects
Development

Successfully merging this pull request may close these issues.

Error in documentation of numba.objmode context manager
4 participants