[BUG] `Location` setter erroring unexpectedly

[henrydingliu]

Are you on the latest chainladder version?

• Yes, this bug occurs on the latest version.

Describe the bug in words

While working on p252 of Friedland, I'm trying to usetri.loc to set some values, but ran into a bug

How can the bug be reproduced?

raa = cl.load_sample('raa')
#`ILocation` sets correctly
raa.iloc[:,:,:,:5] = raa.iloc[:,:,:,:5]
#`Location` errors
raa.loc[:,:,:,:60] = raa.loc[:,:,:,:60]

What is the expected behavior?

this syntax works in pandas

df_dummy = pd.DataFrame({
    'A': [1, 2, 3],
    'B': ['x', 'y', 'z'],
    'C': [0.1, 0.2, 0.3]
})
df_dummy.loc[:,'B':] = df_dummy.loc[:,'B':]

Would you be willing to contribute this ticket?

• Yes, absolutely!
• Yes, but I would like some help.
• No.

[henrydingliu]

i did a bit more digging. there's a sizable gap in complexity between get_idx and __setitem__ in the core _LocBase class. so the key_to_slice results works in get_idx thanks to further processing by _contig_slice, but fails in __setitem__.

@genedan reading back through your slicing tests, i see that you explicitly stated that you are only setting whole triangles. did you see anything in John's old docstrings or comments that indicate he never meant for setting via Location to only work on whole triangles? ultimately, would you support rounding out Location.__setitem__?

[genedan]

If there's a feature you need to get something done, and it's also a legal operation in Pandas, (like this example) you should be able to implement it.

When I come across things that aren't possible/not allowed, I see two (not exhaustive) possible interpretations:

1. Feature hasn't been implemented yet, but will be - only temporarily blocked.
2. Feature cannot be implemented due to irreconcilable breaking changes and other problems

Most of the time, I lean towards the first interpretation. Take for example this snippet:

chainladder-python/chainladder/core/pandas.py

Lines 612 to 625 in 55f9524

	axis: {0 or ‘index’, 1 or ‘columns’}, default 1
	Whether to drop labels from the index (0 or ‘index’)
	or columns (1 or ‘columns’).
	Returns
	-------
	Triangle
	"""
	labels = [labels] if type(labels) is str else list(labels)
	if axis == 1:
	return self[[item for item in self.columns if item not in labels]]
	else:
	raise NotImplementedError("Triangle.drop() only implemented for column axis.")

The docstring was copied over from Pandas, implying 2 axes as acceptable arguments, but the function was only partially implemented for one of them. I interpreted that John just hadn't gotten around to the full implementation rather than not allowing it due to potential problems.

Another example is this one:

chainladder-python/chainladder/core/slice.py

Lines 149 to 150 in 55f9524

	if self.obj.array_backend == "sparse":
	raise ValueError('Setting values with sparse backend requires .at or .iat')

This is a little harder to interpret, but I don't see any reason why we can't eventually implement something that can convert arguments passed to _LocBase.__setitem__() into sparse coords. Even here, I believe it's just a temporary stopgap meant to be addressed later.

So I think we should solve this ticket. core is almost fully covered by tests now, and I think if we ran into problems, we'd find them. I know we fully don't agree on 100% pandas mirroring, but I think this is one of the cases where if we can match pandas more closely, we should. Asking users to read the docs for a non-famous library is a big investment for them, so I think the more we can ease them into the library, the more trust we gain.