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

Remove inplace ops #3

Open
wants to merge 2 commits into
base: py23-more
Choose a base branch
from
Open

Conversation

ChristopherChudzicki
Copy link
Owner

This PR removes numeric in-place operations from expressions.py (string += ops are left in tact).

The motivation for this PR is a change in numpy's behavior, illustrated below:

import numpy as np

A = np.array([1, 1, 1])

A *= 1.5; print(A)
# Numpy 1.6: array([1.5, 1.5, 1.5])
# Numpy 1.16: TypeError: Cannot cast ufunc multiply output from dtype('float64') to dtype('int64') with casting rule 'same_kind'

Prior to numpy 1.10 (I believe), numpy was happy to re-cast arrays to arrays of a different type, in-place.

This becomes a problem for us in situations like:

import numpy as np
from mitxgraders.helpers.calc import evaluator

variables = {
    'A': np.array([1, 1, 1])
}
result, _ = evaluator('A/2', variables, {}, {})
# TypeError: ufunc 'true_divide' output (typecode 'd') could not be coerced to provided output parameter (typecode 'l') according to the casting rule ''same_kind''

Maybe not necessary ...

Really, people shouldn't be using np.array, they should be using our MathArray. It turns out that this change in numpy behavior is not a problem for MathArray since it's "in-place" operations (__iadd__, etc) are fake: they return new objects instead of modifying the original.

Still, this inconsistency in behavior makes me uncomfortable and I'd rather get rid of it. Maybe someone in the future will make MathArray's __iadd__ method truly in-place, or might find a reason to use numpy variables directly, or ...

(Footnote: discoved while trying to fix test https://github.com/mitodl/mitx-grading-library/blob/32a66e19b1ea0b7939e0e88adc9edf4de6a3fe3c/tests/helpers/calc/test_expressions.py#L150)

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.

1 participant