getting a "truth value of an array" error when supplying my own concat_dim.

[WeatherGod]

This bug was introduced sometime after v0.11.0 and has turned up in my test suite using v0.11.2. I'll pass a DataArray() as my concat_dim, and the failure will happen at line 609 in backends/api.py because of: if concat_dim is None or concat_dim == _CONCAT_DIM_DEFAULT:

I am not sure how this change got through. In #2048, I added a unit test that passes a DataArray as a concat_dim argument.

[WeatherGod]

To be more explicit, the issue is that concat_dim == _CONCAT_DIM_DEFAULT is ill-advised because the type of concat_dim is not guaranteed to be a scalar. In fact, the elif of that area of code in api.py explicitly tests if concat_dim is or is not a list.

[shoyer]

Indeed, this is should definitely be concat_dim is _CONCAT_DIM_DEFAULT for comparing to a sentinel value.

I think this snuck through because NumPy will automatically cast size 1 arrays (of any shape) to booleans :(. We should probably adapt that test to include an explicit dimension of size greater than 1.

[WeatherGod]

ah! that's why it snuck through! I have been raking my brain on this for the past hour! shall I go ahead and make a PR?

[WeatherGod]

actually, we could simplify the conditional to be just concat_dim is _CONCAT_DIM_DEFAULT and not bother with the None test.

[shoyer]

I think concat_dim=None may mean something special here, which is why we have _CONCAT_DIM_DEFAULT

[WeatherGod]

scratch that... the test was an or, not a and.

[TomNicholas]

I just wanted to clarify that concat_dim=None very much does mean something special, it means "don't use concat on these datasets at all, just use merge". This is documented under the concat_dim argument description of the docs for auto_combine here.

Also @WeatherGod I think this bug was introduced by me in #2553, but should be checked again once #2616 is finished, because the API will change.