Skip to content

Commit c4388dc

Browse files
committed
Fix the behavior or column SetName method
1 parent c080c46 commit c4388dc

File tree

5 files changed

+98
-12
lines changed

5 files changed

+98
-12
lines changed

src/Microsoft.Data.Analysis/DataFrame.Join.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ private void SetSuffixForDuplicatedColumnNames(DataFrame dataFrame, DataFrameCol
3030
{
3131
// Pre-existing column. Change name
3232
DataFrameColumn existingColumn = dataFrame.Columns[index];
33-
dataFrame._columnCollection.SetColumnName(existingColumn, existingColumn.Name + leftSuffix);
33+
existingColumn.SetName(existingColumn.Name + leftSuffix);
3434
column.SetName(column.Name + rightSuffix);
3535
index = dataFrame._columnCollection.IndexOf(column.Name);
3636
}

src/Microsoft.Data.Analysis/DataFrame.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ public DataFrame AddPrefix(string prefix, bool inPlace = false)
301301
for (int i = 0; i < df.Columns.Count; i++)
302302
{
303303
DataFrameColumn column = df.Columns[i];
304-
df._columnCollection.SetColumnName(column, prefix + column.Name);
304+
column.SetName(prefix + column.Name);
305305
df.OnColumnsChanged();
306306
}
307307
return df;
@@ -316,7 +316,7 @@ public DataFrame AddSuffix(string suffix, bool inPlace = false)
316316
for (int i = 0; i < df.Columns.Count; i++)
317317
{
318318
DataFrameColumn column = df.Columns[i];
319-
df._columnCollection.SetColumnName(column, column.Name + suffix);
319+
column.SetName(column.Name + suffix);
320320
df.OnColumnsChanged();
321321
}
322322
return df;

src/Microsoft.Data.Analysis/DataFrameColumn.cs

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,26 @@ protected set
8484
}
8585
}
8686

87+
// List of ColumnCollections that owns the column
88+
// Current API allows column to be added into multiple dataframes, that's why the list is needed
89+
private readonly List<DataFrameColumnCollection> _ownerColumnCollections = new();
90+
91+
internal void AddOwner(DataFrameColumnCollection columCollection)
92+
{
93+
if (!_ownerColumnCollections.Contains(columCollection))
94+
{
95+
_ownerColumnCollections.Add(columCollection);
96+
}
97+
}
98+
99+
internal void RemoveOwner(DataFrameColumnCollection columCollection)
100+
{
101+
if (_ownerColumnCollections.Contains(columCollection))
102+
{
103+
_ownerColumnCollections.Remove(columCollection);
104+
}
105+
}
106+
87107
/// <summary>
88108
/// The number of <see langword="null" /> values in this column.
89109
/// </summary>
@@ -95,24 +115,30 @@ public abstract long NullCount
95115
private string _name;
96116

97117
/// <summary>
98-
/// The name of this column.
118+
/// The column name.
99119
/// </summary>
100120
public string Name => _name;
101121

102122
/// <summary>
103-
/// Updates the name of this column.
123+
/// Updates the column name.
104124
/// </summary>
105125
/// <param name="newName">The new name.</param>
106-
/// <param name="dataFrame">If passed in, update the column name in <see cref="DataFrame.Columns"/></param>
107-
public void SetName(string newName, DataFrame dataFrame = null)
126+
public void SetName(string newName)
108127
{
109-
if (!(dataFrame is null))
110-
{
111-
dataFrame.Columns.SetColumnName(this, newName);
112-
}
128+
foreach (var owner in _ownerColumnCollections)
129+
owner.UpdateColumnNameMetadata(this, newName);
130+
113131
_name = newName;
114132
}
115133

134+
/// <summary>
135+
/// Updates the name of this column.
136+
/// </summary>
137+
/// <param name="newName">The new name.</param>
138+
/// <param name="dataFrame">Ignored (for backward compatibility)</param>
139+
[Obsolete]
140+
public void SetName(string newName, DataFrame dataFrame) => SetName(newName);
141+
116142
/// <summary>
117143
/// The type of data this column holds.
118144
/// </summary>

src/Microsoft.Data.Analysis/DataFrameColumnCollection.cs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,23 @@ internal IReadOnlyList<string> GetColumnNames()
4141
return ret;
4242
}
4343

44+
public void RenameColumn(string currentName, string newName)
45+
{
46+
var column = this[currentName];
47+
column.SetName(newName);
48+
}
49+
50+
[Obsolete]
4451
public void SetColumnName(DataFrameColumn column, string newName)
52+
{
53+
column.SetName(newName);
54+
}
55+
56+
//Updates column's metadata (is used as a callback from Column class)
57+
internal void UpdateColumnNameMetadata(DataFrameColumn column, string newName)
4558
{
4659
string currentName = column.Name;
4760
int currentIndex = _columnNameToIndexDictionary[currentName];
48-
column.SetName(newName);
4961
_columnNames[currentIndex] = newName;
5062
_columnNameToIndexDictionary.Remove(currentName);
5163
_columnNameToIndexDictionary.Add(newName, currentIndex);
@@ -76,6 +88,9 @@ protected override void InsertItem(int columnIndex, DataFrameColumn column)
7688
{
7789
throw new ArgumentException(string.Format(Strings.DuplicateColumnName, column.Name), nameof(column));
7890
}
91+
92+
column.AddOwner(this);
93+
7994
RowCount = column.Length;
8095
_columnNames.Insert(columnIndex, column.Name);
8196
_columnNameToIndexDictionary[column.Name] = columnIndex;
@@ -99,10 +114,14 @@ protected override void SetItem(int columnIndex, DataFrameColumn column)
99114
{
100115
throw new ArgumentException(string.Format(Strings.DuplicateColumnName, column.Name), nameof(column));
101116
}
117+
102118
_columnNameToIndexDictionary.Remove(_columnNames[columnIndex]);
103119
_columnNames[columnIndex] = column.Name;
104120
_columnNameToIndexDictionary[column.Name] = columnIndex;
121+
122+
this[columnIndex].RemoveOwner(this);
105123
base.SetItem(columnIndex, column);
124+
106125
ColumnsChanged?.Invoke();
107126
}
108127

@@ -114,7 +133,10 @@ protected override void RemoveItem(int columnIndex)
114133
_columnNameToIndexDictionary[_columnNames[i]]--;
115134
}
116135
_columnNames.RemoveAt(columnIndex);
136+
137+
this[columnIndex].RemoveOwner(this);
117138
base.RemoveItem(columnIndex);
139+
118140
ColumnsChanged?.Invoke();
119141
}
120142

test/Microsoft.Data.Analysis.Tests/DataFrameTests.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,44 @@ public void InsertAndRemoveColumnTests()
323323
Assert.True(ReferenceEquals(charColumn, charColumn_1));
324324
}
325325

326+
[Fact]
327+
public void RenameColumnWithSetNameTests()
328+
{
329+
StringDataFrameColumn city = new StringDataFrameColumn("City", new string[] { "London", "Berlin" });
330+
PrimitiveDataFrameColumn<int> temp = new PrimitiveDataFrameColumn<int>("Temperature", new int[] { 12, 13 });
331+
332+
DataFrame dataframe = new DataFrame(city, temp);
333+
334+
// Change the name of the column:
335+
dataframe["City"].SetName("Town");
336+
var renamedColumn = dataframe["Town"];
337+
338+
Assert.Throws<ArgumentException>(() => dataframe["City"]);
339+
340+
Assert.NotNull(renamedColumn);
341+
Assert.Equal("Town", renamedColumn.Name);
342+
Assert.True(ReferenceEquals(city, renamedColumn));
343+
}
344+
345+
[Fact]
346+
public void RenameColumnWithRenameColumnTests()
347+
{
348+
StringDataFrameColumn city = new StringDataFrameColumn("City", new string[] { "London", "Berlin" });
349+
PrimitiveDataFrameColumn<int> temp = new PrimitiveDataFrameColumn<int>("Temperature", new int[] { 12, 13 });
350+
351+
DataFrame dataframe = new DataFrame(city, temp);
352+
353+
// Change the name of the column:
354+
dataframe.Columns.RenameColumn("City", "Town");
355+
var renamedColumn = dataframe["Town"];
356+
357+
Assert.Throws<ArgumentException>(() => dataframe["City"]);
358+
359+
Assert.NotNull(renamedColumn);
360+
Assert.Equal("Town", renamedColumn.Name);
361+
Assert.True(ReferenceEquals(city, renamedColumn));
362+
}
363+
326364
[Fact]
327365
public void TestBinaryOperations()
328366
{

0 commit comments

Comments
 (0)