issue_comments

woohoo!

Thank you @chunweiyuan!

Alright, if that's desired, I'd just change code in apply_over_both() of dataset._calculate_binary_op to react to different kwarg values, and adding a method in test_dataset.py. Should be able to push before the weekend is over.

My concern is how this setting currently does not change how we join variables in Dataset math. Consider the current behavior -- it always does an inner join of the data variables:

``` In [2]: import xarray as xr

In [3]: ds1 = xr.Dataset({'foo': 1, 'bar': 2})

In [4]: ds2 = xr.Dataset({'bar': 2, 'baz': 3})

In [5]: ds1 Out[5]: <xarray.Dataset> Dimensions: () Coordinates: empty Data variables: bar int64 2 foo int64 1

In [6]: ds2 Out[6]: <xarray.Dataset> Dimensions: () Coordinates: empty Data variables: bar int64 2 baz int64 3

In [7]: ds1 + ds2 Out[7]: <xarray.Dataset> Dimensions: () Coordinates: empty Data variables: bar int64 4 ```

Logically, it makes sense for this option to switch this to an outer join instead.

Just checking in. Is there something else that needs to be added to this PR before merge?

It would be good to ensure that this affects the graph and that the actual tasks don't depend on the global state.

Indeed, we are safe here. align is turned into a bunch of NumPy or dask.array calls, so this option will simply effect how the graph is built.

I haven't noticed people getting caught by using set_options from multiple threads. It definitely isn't threadsafe but it's usually only used when constructing computations, which is often done from a single thread.

It would be good to ensure that this affects the graph and that the actual tasks don't depend on the global state.

It occurs to me now that the other reason why people avoid global variables is because of concerns around thread safety and distributed computation. Even with the context manager, bad things will happen if you use set_options simultaneously from different threads.

I need to think about this a little more. Copious warnings in the docs might be enough, or maybe entering a set_options context should also enter an explicit threading.Lock() context.

On the other hand, the current implementation is basically identical to how functionality like NumPy's np.errstate() works, and people seem to be pretty happy with that.

@mrocklin does this seem like the sort of change we can do in xarray without seriously regretting it later? Xarray uses basically the same set_options machinery that dask has.

Set coordinates on the 1D -- something like [0,1,2] for the first, [1,2,3] for the second. Then the inner join should have [1,2] vs [0,1,2,3] for outer join. On Fri, Oct 28, 2016 at 12:00 PM chunweiyuan notifications@github.com wrote:

Out of curiosity, how do I test inner-join vs. outer-join using only 1-D arrays? I can certainly simplify the current tests to be 2D + 1D.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/1065#issuecomment-257001251, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1qyX-HigWTUAJnBuCpR4AqNFzyYaks5q4kXigaJpZM4Ki4oh .

Out of curiosity, how do I test inner-join vs. outer-join using only 1-D arrays? I can certainly simplify the current tests to be 2D + 1D.

Let's find a more descriptive name for this option than merely join. Maybe: compute_join, compute_align, arithmetic_join, math_join, ...?

Also please add documentation notes about this new functionality to this section of the docs and the "what's new" page.