Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

describe() fixes #937

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

describe() fixes #937

wants to merge 15 commits into from

Conversation

Jolanrensen
Copy link
Collaborator

@Jolanrensen Jolanrensen commented Oct 29, 2024

Fixes #352 and part of #558

I'll first fix some of the most annoying bugs which can be found with describe(), more changes or fixes will come later.

Fixed:

  • created col.isInterComparable() instead of col.isComparable() to avoid confusion
  • cumsum:
    • added Short, Byte, BigInteger, Nothing support
    • (Short/Byte are supposed to become Int, similar to sum(), but they are temporarily converted back to Short/Byte so that the DataColumn.cumSum function returns the right type)
    • added extra tests
  • mean:
    • added BigInteger support
    • added TODOs for BigDecimal and BigInteger, they're supposed to result in BigDecimal, not Double
  • median:
    • added Float, Short, BigInteger support
    • Number support is still missing as it's not Comparable, will be handled later
    • added TODOs for conversion, will happen later
  • min/max:
    • removed unused internal code
    • Number support is still missing as it's not Comparable, will be handled later
  • std:
    • added BigInteger, Number support
    • added test for Byte
    • BigDecimal and BigInteger are supposed to result in BigDecimal, not Double, will be handled later
  • sum:
    • added Float, Short, Byte, BigInteger, Number, Nothing support
    • added some tests
    • conversions will be handled later
  • describe:
    • fixed @DataSchema type being String instead of KType
    • some cleaning up in the implementation
    • min, median, and max now also show up for Number columns with different types :) Numbers will be converted to double or BigDecimal first so they can be compared.

@Jolanrensen Jolanrensen force-pushed the statistics-fixes branch 2 times, most recently from dc4687d to 6e028f1 Compare November 7, 2024 13:56
…ng them to either double or bigdecimal) and added tests
@Jolanrensen Jolanrensen changed the title Statistics fixes describe() fixes Nov 8, 2024
@Jolanrensen Jolanrensen marked this pull request as ready for review November 15, 2024 12:20
@Jolanrensen Jolanrensen requested review from zaleslaw and koperagen and removed request for zaleslaw November 15, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading exception message in "sum" aggregator
1 participant