DYN-8693 Fix List.SortIndexByValue to accept integers and mixed numeric types#17053
DYN-8693 Fix List.SortIndexByValue to accept integers and mixed numeric types#17053Copilot wants to merge 3 commits into
Conversation
…rable types Agent-Logs-Url: https://github.com/DynamoDS/Dynamo/sessions/b0cc73a2-9ed4-4556-bc2b-38de3c0f3bae Co-authored-by: johnpierson <15744724+johnpierson@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8693
johnpierson
left a comment
johnpierson
left a comment
There was a problem hiding this comment.
looks awesome to me mr @copilot
|
There was a problem hiding this comment.
Pull request overview
Updates DSCore.List.SortIndexByValue to support integer and mixed numeric inputs (and other object-typed values) by switching to the shared ObjectComparer, aligning behavior with DSCore.List.Sort.
Changes:
- Changed
SortIndexByValueinput fromList<double>toIEnumerable<object>and replaced the custom sorting logic withObjectComparer. - Removed a redundant/no-op
OrderByfrom the prior implementation. - Updated and expanded unit tests to cover integer and mixed numeric inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Libraries/CoreNodes/List.cs |
Broadens accepted input types and uses ObjectComparer for consistent cross-type ordering. |
test/Libraries/CoreNodesTests/ListTests.cs |
Updates existing test to List<object> and adds new cases for integer and mixed numeric inputs. |
| public static IEnumerable SortIndexByValue(IEnumerable<object> list) | ||
| { | ||
| List<Tuple<int, double>> tupleList = new List<Tuple<int, double>>(); | ||
| for (int i = 0; i < list.Count; i++) | ||
| { | ||
| tupleList.Add(new Tuple<int, double>(i, list[i])); | ||
| } | ||
| tupleList = tupleList.OrderBy(x => x.Item2).ToList(); | ||
| IEnumerable<int> newList = tupleList.OrderBy(x => x.Item2).Select(y => y.Item1); | ||
| IEnumerable<int> newList = list | ||
| .Select((value, index) => new Tuple<int, object>(index, value)) | ||
| .OrderBy(x => x.Item2, new ObjectComparer()) | ||
| .Select(y => y.Item1); |
Copilot
AI
Apr 15, 2026
•
There was a problem hiding this comment.
Changing the parameter type from List<double> to IEnumerable<object> is a source/binary breaking change for any compiled callers (e.g., List<double>/IEnumerable<double> can’t be passed to IEnumerable<object> because covariance doesn’t apply to value types). Consider preserving the old signature (e.g., keep SortIndexByValue(List<double>) and delegate to the new implementation, or change the new API to accept non-generic IEnumerable/IList for broader compatibility) to avoid breaking external consumers while still fixing Dynamo’s type coercion issue.
There was a problem hiding this comment.
you might want to also consider the performance and memory impact of this change on large lists.
There was a problem hiding this comment.
Changing the parameter type from
List<double>toIEnumerable<object>is a source/binary breaking change for any compiled callers (e.g.,List<double>/IEnumerable<double>can’t be passed toIEnumerable<object>because covariance doesn’t apply to value types). Consider preserving the old signature (e.g., keepSortIndexByValue(List<double>)and delegate to the new implementation, or change the new API to accept non-genericIEnumerable/IListfor broader compatibility) to avoid breaking external consumers while still fixing Dynamo’s type coercion issue.
@copilot do this.
There was a problem hiding this comment.
Changed the parameter to IList (non-generic) in b69fa14. This is backward-compatible — List<double>, List<int>, List<object> all implement IList, so no breaking change for existing compiled callers. This is also the dominant pattern used by virtually every other method in List.cs.
On performance: IList.Cast<object>() produces a lazy iterator (no extra allocation beyond a single thin wrapper), so the memory and time complexity remain the same O(n log n) / O(n) as before — only the boxing cost for value-type elements is added, which is equivalent to what List.Sort(IEnumerable<object>) does.
Also fixed the stable-sort test assertions for equal-value inputs ({ 8, 4, 2, 4, 0 } → [4, 2, 1, 3, 0], not [4, 2, 3, 1, 0]) and updated the integer test to use List<int> directly.
…t assertions Agent-Logs-Url: https://github.com/DynamoDS/Dynamo/sessions/6ce82cbe-6eef-4038-8232-10aa356ec69f Co-authored-by: johnpierson <15744724+johnpierson@users.noreply.github.com>
Purpose
Fixes
List.SortIndexByValuesilently failing when passed integers or mixed numeric lists. The parameter was typed asList<double>, causing Dynamo's type coercion to fail on common use cases. Resolves DYN-8693.Declarations
Check these if you believe they are true
Release Notes
Fixed
List.SortIndexByValueto correctly accept integers and mixed numeric types (int, double, float). Previously, passing integers would fail silently due to type coercion limitations withList<double>.Reviewers
@johnpierson
FYIs
N/A