SparseSeries.combine_first(Series) fails when combining with a Series

[creeson]

Contrary to the documentation, the SparseSeries code, when doing a combine_first, actually assumes the other series is a SparseSeries. It performs a other.to_dense(), which fails on a Series.

def combine_first(self, other):
    """
    Combine Series values, choosing the calling Series's values
    first. Result index will be the union of the two indexes

    Parameters
    ----------
    other : Series

    Returns
    -------
    y : Series
    """
    dense_combined = self.to_dense().combine_first(other.to_dense())
    return dense_combined.to_sparse(fill_value=self.fill_value)

I changed my code locally to the following, which seems to work (though I'm sure there is a more elegant way):

def combine_first(self, other):
    """
    Combine Series values, choosing the calling Series's values
    first. Result index will be the union of the two indexes

    Parameters
    ----------
    other : Series

    Returns
    -------
    y : Series
    """
    if isinstance(other, SparseSeries):
        dense_combined = self.to_dense().combine_first(other.to_dense())
    elif isinstance(other, Series):
        dense_combined = self.to_dense().combine_first(other)
    return dense_combined.to_sparse(fill_value=self.fill_value)

[wesm]

Good point. I also noticed that SparseDataFrame.combine_first is broken and had no unit test using it so I fixed that also in the above commit. The sparse data structures could use a lot more testing in general to make sure the API is reasonably compliant