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.

Sign up for GitHub

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

Jump to bottom

Clean up Analytic Functions #621

Merged
merged 7 commits into from
Aug 25, 2019
Merged

Clean up Analytic Functions #621

merged 7 commits into from
Aug 25, 2019

Conversation

ryancerf
Copy link
Collaborator

@ryancerf ryancerf commented Aug 21, 2019 •
edited
Loading

Thanks for contributing.

Description

  • Added first pass of JavaDoc.
  • Made as many of the classes/methods package private as possible.
  • Added comments explaining implementation.
  • Implemented executeInPlace method.
  • cleaned up sliding vs append windows. Do not allow append windows to override removeLeftMost.
  • Use consistent naming for referring to left and right side of a window.
  • Implemented remaining basic analytic functions min, mean and count

Continues work on #582

Testing

Yes added and improved testing.

= added 2 commits August 21, 2019 13:39
* Improve JavaDoc for public classes and methods.
f50a982 
* Add comments explaining the implemention.
* Make as much of the API as possible package private.
* Clean up the names used to describe windows (left and right).
Implement executeInPlace
3c744b0
@ryancerf ryancerf requested a review from lwhite1 August 21, 2019 21:25
lwhite1
lwhite1 reviewed Aug 22, 2019
View reviewed changes
Copy link
Collaborator

@lwhite1 lwhite1 left a comment

Choose a reason for hiding this comment

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

I'm only about 1/3 of the way through, and will come back to it in the morning. Looks like you've made a lot of progress.

core/src/main/java/tech/tablesaw/analytic/AggregateFunction.java Outdated Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AggregateFunction.java Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AggregateFunctions.java Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AggregateFunctions.java Outdated Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AggregateFunctions.java Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AggregateFunctions.java Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AnalyticQuery.java Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AnalyticQuery.java Outdated Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AnalyticQuery.java Show resolved Hide resolved
lwhite1
lwhite1 reviewed Aug 22, 2019
View reviewed changes
Copy link
Collaborator

@lwhite1 lwhite1 left a comment

Choose a reason for hiding this comment

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

Minor changes and a few questions. otherwise looks good.

core/src/main/java/tech/tablesaw/analytic/AnalyticQueryEngine.java Outdated Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AnalyticQuerySteps.java Outdated Show resolved Hide resolved
core/src/main/java/tech/tablesaw/analytic/AnalyticQuerySteps.java Show resolved Hide resolved
@ryancerf
Copy link
Collaborator Author

Thanks for the review.

@lwhite1 lwhite1 merged commit 4e37821 into jtablesaw:master Aug 25, 2019
@ryancerf ryancerf deleted the analytic branch September 3, 2019 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lwhite1 lwhite1 lwhite1 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryancerf @lwhite1