[bot] Merge master/56f00e1f into rel/dev - #1693
Merged
Merged
Conversation
The JSON extraction path converted attribute values one at a time, calling pandas.to_datetime / pandas.to_numeric per scalar and re-resolving the converter for every value. Convert each attribute column in a single vectorized call instead: parse each raw value with the cheap per-value to_type(), then apply the pandas conversion once to the whole column. This is safe because to_type() fully parses the raw strings first, so the vectorized call only ever wraps already-typed date/datetime/int objects (never raw strings, which could be subject to column-wide format inference). Measured 6-13x faster attribute-column conversion on 50k-row columns. JIRA: CQ-2677 risk: low
…ot per page The paging loop resolved each index / attribute column to its CatalogAttribute via a linear catalog scan (_find_attribute) on every page. The Attribute -> CatalogAttribute mapping is invariant across pages, so resolve it once before the loop and reuse it. JIRA: CQ-2677 risk: low
The Arrow conversion path ran four functions (convert_arrow_table_to_dataframe, compute_row/column_totals_indexes, compute_primary_labels) on the same table, each independently orjson-parsing the schema metadata blobs. Parse the schema metadata once in the orchestrator and pass it into the functions via an optional schema_meta parameter. The parsed metadata is treated as read-only throughout, so sharing one instance is safe. JIRA: CQ-2677 risk: low
reorder_grand_totals and compute_row_totals_indexes both figured out which rows are totals by walking the __row_type column value by value in Python. Letting pyarrow/numpy compare the whole column at once is faster and keeps the code simpler. JIRA: CQ-2677 risk: low
The Arrow indexed()/not_indexed() extraction (use_arrow=True) converted every metric column into a Python list via to_pylist(), which pandas.DataFrame then re-parsed back into a float64 block - an Arrow -> Python objects -> pandas round-trip. Return metric columns as numpy arrays (table.column(...).to_numpy(zero_copy_only=False)) so pandas consumes the numeric block directly. Measured ~930x faster for 200 columns x 100k rows. JIRA: CQ-2677 risk: low
convert_label_values converted date-granularity attribute columns one value at a time via pandas.to_datetime per scalar - the same per-scalar pitfall #1 fixed on the JSON path, and the last significant gap making the Arrow indexed/not_indexed path slower than JSON for date-heavy results. For DateConverter/DatetimeConverter, parse each value with the cheap per-value to_type() and then run one pandas.to_datetime() over the whole column, restoring None for null rows to preserve the previous behaviour. WEEK/QUARTER and other granularities keep the straightforward per-value path. Measured ~13x faster on a 100k-row DAY column; identical values (verified across all granularities, nulls and empty input); covered by the existing convert_label_values and use_arrow tests. JIRA: CQ-2677 risk: low
…t values Building the primary-label mapping for a crosstab used to walk every row for each attribute level and pull the whole label column into Python, even though the mapping only ever needs the distinct values. Instead, filter down to the data rows once and, when the primary and display labels are the same (the usual case), build the map straight from the column's distinct values, so repeated attribute values collapse up front. That is roughly 30x faster for low-cardinality attributes - the typical pivot - and no slower when every value is unique. When the primary and display labels differ, the values are still paired up row by row. The filter also keeps only the label columns it actually needs; otherwise it would copy every metric column along with them, which is a lot of wasted memory on wide results. The output is unchanged, checked against the primary-label fixtures at both low and high cardinality. JIRA: CQ-2677 risk: low
Two targeted guards for the perf changes in this branch, each verified by mutation to fail on the bug it protects against: - _compute_primary_labels_from_inline with metric/grand-total columns and a total row present (primaryLabelId != labelId). Guards the column-projection optimization: a projection that drops a needed label column, or a filter that leaks total rows, would break it - and the existing separate-column test cannot catch either (its table has no extra columns and no total row). - _typed_attribute_values date-granularity batching. The JSON exec_def cassettes only contain text attributes (granularity: null), so the vectorized date conversion was otherwise unexercised. JIRA: CQ-2677 risk: low
The ty type checker cannot see pyarrow.compute functions (equal, not_equal, any) because pyarrow generates them at runtime, so it reported them as unresolved attributes. Mark those call sites with ty: ignore[unresolved-attribute] and extract the masks into named locals so the lines stay within the format limit and read clearly. No behavior change. JIRA: CQ-2677 risk: low
perf(gooddata-pandas): speed up and slim down DataFrame conversion
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rel/dev #1693 +/- ##
===========================================
+ Coverage 78.02% 78.05% +0.02%
===========================================
Files 271 271
Lines 18639 18669 +30
===========================================
+ Hits 14544 14572 +28
- Misses 4095 4097 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Automated PR to perform merge from master into rel/dev with changes up to 56f00e1 (created by https://github.com/gooddata/gooddata-python-sdk/actions/runs/29727223381).