Skip to content

Fix bug in groupby checking wrong colDtype#398

Merged
steveoni merged 1 commit into
javascriptdata:devfrom
igonro:fix-groupby-bug
Feb 23, 2022
Merged

Fix bug in groupby checking wrong colDtype#398
steveoni merged 1 commit into
javascriptdata:devfrom
igonro:fix-groupby-bug

Conversation

@igonro

@igonro igonro commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

Closes #396

  • Write test that would fail in src/danfojs-browser/tests/aggregators/groupby.test.js
  • Write test that would fail in src/danfojs-node/test/aggregators/groupby.test.ts
  • Modify src/danfojs-base/aggregators/groupby.ts and src/danfojs-base/core/frame.ts to fix the issue
  • Run the tests again and check that the tests passed

Feel free to modify the code or add comments with the changes if you think something could be improved.

Thanks!

@steveoni steveoni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@steveoni steveoni merged commit 5d8bd2d into javascriptdata:dev Feb 23, 2022
let colDtype = this.colDtype[colIndex]
if (colDtype === "string") throw new Error(`Can't perform math operation on column ${colName}`)
let operationVal = (typeof operation === "string") ? operation : operation[colName]
if (colDtype === "string" && operationVal !== "count") throw new Error(`Can't perform math operation on column ${colName}`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yay! This will fix a problem I ran into.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants