Repository: numpy/numpy
Pull request: DOC: add note on type casting to numpy.left_shift().
It is not obvious how numpy.left_shift() infers types and this may lead
to unexpected outputs.  Added note to draw attention to the importance
of input types.  Added three examples.  "Closes #12310"

<!-- Please be sure you are following the instructions in the dev guidelines
http://www.numpy.org/devdocs/dev/development_workflow.html
-->

<!-- We'd appreciate it if your commit message is properly formatted
http://www.numpy.org/devdocs/dev/development_workflow.html#writing-the-commit-message
-->


Problem statement:
np.left_shift and np.right_shift should infer types from second arg
<!-- Please describe the issue in detail here, and fill in the fields below -->

`np.left_shift` and `np.right_shift` shouldn't change the dtype of the first argument to match that of the second argument. It took me a few hours to relearn bit shifting before realizing that it was NumPy's fault and why there was such behavior.

<!-- A short code example that reproduces the problem/missing feature. It should be
self-contained, i.e., possible to run as-is via 'python myproblem.py' -->

```python
import numpy as np
np.left_shift(np.uint8(255), 1) # 510 - incorrect
np.left_shift(np.uint8(255), np.uint8(1)) # 254 - correct
```


<!-- Output from 'import sys, numpy; print(numpy.__version__, sys.version)' -->
1.15.1 3.7.0 (default, Jun 28 2018, 07:39:16) 
[Clang 4.0.1 (tags/RELEASE_401/final)]

Hints from issue discussion:
Inferred dtypes do depend generally on both components, and it is not necessarily obvious what the preferred answer should be -- 510 is correct if you think of `np.uint(255)` just as a very compact way to store the integer 255. 

But I do agree it is confusing, especially since if the first argument is an array, its dtype does get precedence, but if both are arrays, the "common dtype" is used:
```
np.left_shift(np.array([255], dtype='u1'), 1)
# array([254], dtype=uint8)
np.left_shift(np.array([255], dtype='u1'), np.array([1]))
# array([510])
```

In the end, we cannot easily change it, as it would break more than it is worth; probably the best we can do is to try to document this more clearly. So, perhaps the question is how the documentation could have helped to find out more quickly?
@mhvk I agree with your assessment. I think the documentation should include disambiguating examples such as the ones we listed.
Re-opening and changing title since this now is a request for improved documentation.
@mhvk Hi, I'm a student looking to get into open source, I'm interested in contributing to this feature, can you assign it to me and give me some steps in the right direction?
@mattpaletta - just to be sure: this has become a request for documentation, which is why it is labelled as easy. The ufunc docstrings are assigned in `/numpy/core/code_generators/ufunc_docstrings.py` - the idea would be to clarify how things work there.
@mhvk That's no problem. I believe I understand the ambiguity, and can write appropriate documentation to make it more clear for users.
@mhvk So is it because that as the second array is of dtype i4 and i4 is given more precedence to u1 , we are getting 510 as the answer?
@Sedherthe - the general idea is that when two arrays with different dtype are involved, they both get converted to a new dtype that can represent values in either. See the ufunc [casting rules](https://docs.scipy.org/doc/numpy/reference/ufuncs.html#casting-rules). Hence,
```
In [3]: np.left_shift(np.array([255], dtype='u1'), np.array([1], dtype='u1'))
Out[3]: array([254], dtype=uint8)

In [4]: np.left_shift(np.array([255], dtype='u1'), np.array([1], dtype='i1'))
Out[4]: array([510], dtype=int16)
```

Note that this is all about the *inputs*:
```
In [8]: np.left_shift(np.array([255], dtype='u1'), np.array([1], dtype='i1'), casting='no')
TypeError: ufunc 'left_shift' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''no''
```

Now, when a scalar is passed in, the difference is that it gets checked whether the scalar can be represented as the dtype of the other (see end of link above); if it can, it gets converted to that dtype (even if that means the *result* of the operation might not fit in that dtype, as in the example on top).

Anyway, for the documentation, the idea would be to link to the casting rules and present some examples.
@mhvk Okay, Got it.Thank you for the explanation!
I went through the result_type, promote_type and min_scalar_type references as well, which i felt were useful for complete understanding!
I want to help in this issue! Can I contribute to this!?

@Sedherthe - yes, if you can add some documentation, that would be great. The trick will be to get the balance right between being clear and not making it so long that it distracts from the main points.

Commit message: Made changes as suggested by rossbar in PR

Used the examples of zhanwenchen

Patch to review:
diff --git a/numpy/core/code_generators/ufunc_docstrings.py b/numpy/core/code_generators/ufunc_docstrings.py
index 1295166585bb..b2de386f6d9e 100644
--- a/numpy/core/code_generators/ufunc_docstrings.py
+++ b/numpy/core/code_generators/ufunc_docstrings.py
@@ -1835,6 +1835,15 @@ def add_newdoc(place, name, doc):
     >>> np.left_shift(5, [1,2,3])
     array([10, 20, 40])
 
+    Note that the dtype of the second argument may change the dtype of the
+    result and can lead to unexpected results in some cases (see
+    :ref:`Casting Rules <ufuncs.casting>`):
+
+    >>> a = np.left_shift(np.uint8(255), np.uint8(1)); print(a, type(a))
+    254 <class 'numpy.uint8'> # as expected
+    >>> b = np.left_shift(np.uint8(255), 1); print(b, type(b))
+    510 <class 'numpy.int64'> # unexpected
+
     """)
 
 add_newdoc('numpy.core.umath', 'less',


Code review comments:
diff_hunk: @@ -1835,6 +1835,15 @@ def add_newdoc(place, name, doc):
     >>> np.left_shift(5, [1,2,3])
     array([10, 20, 40])
 
+    Note that the dtype of the second argument may change the dtype of the
+    result and can lead to unexpected results in some cases (see
+    :ref:`Casting Rules <ufuncs.casting>`):
+
+    >>> a = np.left_shift(np.uint8(255), np.uint8(1)); print(a, type(a))
+    254 <class 'numpy.uint8'> # as expected
+    >>> b = np.left_shift(np.uint8(255), 1); print(b, type(b))
+    510 <class 'numpy.int64'> # unexpected
path: numpy/core/code_generators/ufunc_docstrings.py
text: @user1:
```suggestion
    >>> a = np.left_shift(np.uint8(255), 1) # Expect 254
    >>> print(a, type(a)) # Unexpected result due to upcasting
    510 <class 'numpy.int64'>
    >>> b = np.left_shift(np.uint8(255), np.uint8(1))
    >>> print(b, type(b))
    254 <class 'numpy.uint8'>

```