NCAS-CMS / NCAS-CMS/cf-python

Dubious type casting for augmented arithmetic operators

Open
#435 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

question testing
Dominant language
Python
Stars
150
Forks
23
Avg merge
1d 11h
Merged PRs (30d)
2

Description

We currently allow, and validate via testing in test_Data_BINARY_AND_UNARY_OPERATORS, some behaviour relating to input and output data types for augmented arithmetic assignment operators that is not allowed by NumPy, and we should consider whether this is suitable or not. I am inclined to say we should make appropriate changes to adopt the NumPy behaviour.

Specifics

Namely, when an augmented assignment is performed using inputs with data types which lead to a change in data type for the output, e.g. for a simplified scalar case something like a = 1; a += 1.0, we permit an in-place change of array dtype. As a minimal example, note how NumPy raises a type casting error for the equivalent operation below, whereas we go ahead and produce an output with a changed data type, the same type that the operation not in-place would produce:

>>> import cf
>>> import numpy as np
>>> 
>>> # Setup equivalent arrays
>>> i_np = np.array([1, 2, 3])
>>> i_cf = cf.Data(i_np)
>>> 
>>> # NumPy raises a type casting error:
>>> i_np + 1.0  # operation not in-place is fine
array([2., 3., 4.])
>>> i_np += 1.0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
numpy.core._exceptions.UFuncTypeError: Cannot cast ufunc 'add' output from dtype('float64') to dtype('int64') with casting rule 'same_kind'
>>> 
>>> # ... whereas cf performs the operation to give the same result data type
>>> # as the non in-place operation would:
>>> i_cf + 1.0
<CF Data(3): [2.0, 3.0, 4.0]>
>>> i_cf += 1.0
>>> i_cf
<CF Data(3): [2.0, 3.0, 4.0]>

and the equivalent behaviour occurs for the various __i<operator>__ operators.

Relevant cases in test suite

For reference, the tests in test_Data_BINARY_AND_UNARY_OPERATORS which were checking for this (dubious) behaviour, which remain as such from before the LAMA to Dask migration, are:

https://github.com/NCAS-CMS/cf-python/blob/003374315b37568f8594db0bbbab3b5d5c31e0b8/cf/test/test_Data.py#L2062-L2154

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading and running the referenced cases in cf/test/test_Data.py, especially test_Data_BINARY_AND_UNARY_OPERATORS around lines 2062–2154, and reproduce the NumPy comparisons in the issue. Done means the desired augmented-assignment casting policy is decided and the implementation and tests consistently reflect it.

Written by the indexing model from the issue text.

Assessment

Tech stack
numpy, python
Domain
data
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.