Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 28 additions & 17 deletions chainladder/core/pandas.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def append(self, other):
def rename(
self,
axis: Literal['index', 'columns', 'origin', 'development'] | int,
value: list | str
value: list | str | dict
) -> Self:
"""Alter axes labels.

Expand All @@ -364,29 +364,40 @@ def rename(
A value of 0 <= axis <= 4 corresponding to axes 'index',
'columns', 'origin', 'development' respectively. Both the
int and str representation can be used.
value: list or str
value: list or str or dict
List of new labels to be assigned to the axis. List must be of
same length of the specified axis.
same length of the specified axis. Can also be a dictionary for renaming columns

Returns
-------
Triangle with relabeled axis.
"""
value = [value] if type(value) is str else value
if axis == "index" or axis == 0:
self.index = value
elif axis == "columns" or axis == 1:
self.columns = value
elif axis == "origin" or axis == 2:
self.origin = value
elif axis == "development" or axis == 3:
self.development = value

if type(value) is dict:
if axis == "columns" or axis == 1:
full_dict = dict(zip(self.columns.values,self.columns.values))
full_dict.update(value)
self.columns = self.columns.map(full_dict)
else:
raise ValueError(
"Invalid value provided to the 'value' parameter. Accepted values for index, origin, and development axes are a str or a list"
)
else:
raise ValueError(
"Invalid value provided to the 'axis' parameter. Accepted values are a string of 'index', "
"'columns', 'origin', or 'development', or an integer in the interval [0, 4] specifying the"
" axis to be modified."
)
value = [value] if type(value) is str else value
if axis == "index" or axis == 0:
self.index = value
elif axis == "columns" or axis == 1:
self.columns = value
elif axis == "origin" or axis == 2:
self.origin = value
elif axis == "development" or axis == 3:
self.development = value
else:
raise ValueError(
"Invalid value provided to the 'axis' parameter. Accepted values are a string of 'index', "
"'columns', 'origin', or 'development', or an integer in the interval [0, 4] specifying the"
" axis to be modified."
)
return self

def astype(self, dtype, inplace=True):
Expand Down
13 changes: 13 additions & 0 deletions chainladder/core/tests/test_triangle.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,20 @@ def test_rename_columns(genins, clrd) -> None:
# Test the cascading of rename to triangle.columns_label.
assert genins.columns_label == ['foo']

genins.rename('columns',{'foo':'newfoo'})

assert genins.columns.to_list() == ['newfoo']

genins.rename('columns',{'foo':'newnewfoo'})

assert genins.columns.to_list() == ['newfoo']


def test_rename_exception(genins, clrd) -> None:
# Test incorrect value argument - misspelling of string.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I understood this comment. To me it seems to be testing 2 things at once, trying to map a new name to a non-existing column, and attempting to do a rename meant for columns, but accidentally applied to the origin instead, and then alerting the user by raising an error.

However, doing something similar in pandas doesn't actually cause an error. For example, if we create a DataFrame with columns A and B but pass a mapping {'C': 'bar'} whose key not only does not match either column but also gets accidentally passed to the index parameter, we just get a copy of the original DataFrame with no error.

import pandas as pd
df = pd.DataFrame({"A": [1, 2, 3], "B": [4, 5, 6]})
df
Out[4]: 
   A  B
0  1  4
1  2  5
2  3  6
df.rename(index={'C': 'bar'})
Out[5]: 
   A  B
0  1  4
1  2  5
2  3  6

I believe this was intentional by design for pandas, for example if we want to apply a mapping where a DataFrame only hit a subset of the keys (or even none), we'd still want it to work.

Eventually, to match the functionality of DataFrame.rename, we'll need to remove the value parameter and supply the mapping to other parameters, working our way towards matching the signature of DataFrame.rename:

DataFrame.rename(mapper=None, *, index=None, columns=None, axis=None, copy=<no_default>, inplace=False, level=None, errors='ignore')

But I remember we talked about moving these further changes to other issues. This PR still makes sense given the chainladder's current behavior, so I can approve this as long as we're aligned that the function's signature will change to match DataFrame.rename (or get as close as possible) in future PRs. Alternatively, if you'd like to tackle #662 in this PR, or even go all the way to get the signatures to match in this PR, I can review that too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not following. this test was named test_rename_exception. since i added a new exception to the rename function, I added a clause to test the new exception.

what would you like to see changed?

with pytest.raises(ValueError):
genins.rename('origin', {'oldfoo':'foo'})

# Test incorrect axis argument - misspelling of string.
with pytest.raises(ValueError):
genins.rename('colunms', 'foo')
Expand Down