issue_comments

OK, I have stripped the Dataset/Array methods which I agree were largely redundant. Since this sets this type of comparison/merge slightly apart, 'no_conflicts' seems the more intuitive wording when used only as a compat option so I've changed it to that.

And I've done a first pass at updating the docs.

I think this is nearly ready. We just need to decide on the final keyword name (e.g., notnull_equals or no_conflicts) and add mention of this in the documentation / what's new.

I'm not sure we really need this new method for Dataset/DataArray (for the current implementation, adding it on Variable would be enough). Given that the user can catch MergeError, it seems straightforward enough just call merge to see if merging is safe.

One potential concern here is that performance is not going to be so great if you attempt to combine a bunch of variables with lazy data loaded with dask, because each comparison will trigger a separate computation. To that end, it would be nice to do the safety check in a single dask operation.

I will have a look into how to do this, but am not that familiar with dask.

Ah sorry - yes rebased locally then mistakenly merged the remote fork...

compat='no_conflicts' is possibly a better keyword argument.

Yes I thought that might be better also, the advantages of 'notnull_equals', (slightly more precise operational description and slightly better 'grammar' when using as a method: if ds1.notnull_equals(ds2): ... vs if ds1.no_conflicts(ds2): ...) are probably negligible since 'no_conflicts' is more intuitive for merge - and this is likely the main usage.

One of the commits in this PR is not yours. Take a look here -- you want to rebase instead of merge (yes git is confusing): http://stackoverflow.com/questions/5968964/avoid-unwanted-merge-commits-and-other-commits-when-doing-pull-request-on-github

I think you can still safely rebase now.

compat='no_conflicts' is possibly a better keyword argument.

Current version is conservative in that it still requires aligned Datasets with all the same variables.

:+1: it's much less obvious to treat missing variables as "missing data."

Due to the float nature of NaN, type is sometimes not preserved, e.g. merging two int arrays yields a float array, even when the final array has no NaN values itself.

Yes, this is consistent with the xarray/pandas type system. (Which is not ideal, but that's another bigger issue.)

This is quite clever! I'm impressed this was so easy using fillna.

One potential concern here is that performance is not going to be so great if you attempt to combine a bunch of variables with lazy data loaded with dask, because each comparison will trigger a separate computation. To that end, it would be nice to do the safety check in a single dask operation.

To be fair, this is already an issue if you're trying to merge lots of datasets together. But I expect this will become more of an issue when there are useful ways to merge a bunch of datasets, which is what this PR enables.