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 numeric type arithmetics #597

Merged
merged 4 commits into from
Jun 21, 2017

Conversation

greschd
Copy link
Member

@greschd greschd commented Jun 19, 2017

Fixes #596.

The two decorators _left_operator and _right_operator handle getting the .value and checks for left-side and right-side operator, respectively.

For the arithmetic itself, the base Python types are used, and then converted back into AiiDA types using to_aiida_type. This function uses singledispatch to select the right function corresponding to the type of the value.

I removed the inline operations (__iadd__ etc.) because that doesn't really go well if the type of the return value could be changed. It would be possible to implement, but IMO doesn't give much benefit. The current version is consistent with the behavior of the corresponding Python types, which are immutable. The operations (+= etc.) can still be used, since Python just falls back on __add__.

Another detail is that the comparisons now return Bool, not bool. This should be fine since the Bool now converts to bool correctly, but it could be changed back if needed.

Side note: I think that to_aiida_type should also be used to automatically infer at least the basic types when passing arguments to processes. Having to explicitly call Str, Int etc. on all inputs is just unnecessary clutter.

@giovannipizzi giovannipizzi requested a review from sphuber June 20, 2017 20:10
@giovannipizzi giovannipizzi merged commit b47154f into aiidateam:develop Jun 21, 2017
@greschd greschd deleted the fix_numeric_type_arithmetics branch August 10, 2017 14:46
@sphuber sphuber added this to the v0.9.1 milestone Aug 31, 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.

3 participants