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

(dsl): Support Weight Avg aggregation #358

Merged

Conversation

vanjaftn
Copy link
Contributor

@vanjaftn vanjaftn commented Nov 8, 2023

Part of #118


If you want to add aggregation (on the same level), you can use `withAgg` method:
```scala
val multipleAggregations: MultipleAggregations = weightedAvgAggregation(name = "weightedAvgAggregation", valueField = Document.intField, weightField = Document.doubleField).withAgg(weightedAvgAggregation(name = "weightedAvgAggregation", valueField = Document.doubleField, weightField = Document.intField))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you give them different names?


If you want to change the `valueMissing`, you can use `valueMissing` method:
```scala
val aggregationWithMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).valueMissing(10.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val aggregationWithMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).valueMissing(10.0)
val aggregationWithValueMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).valueMissing(10.0)


If you want to change the `weightMissing`, you can use `weightMissing` method:
```scala
val aggregationWithMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(5.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val aggregationWithMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(5.0)
val aggregationWithWeightMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(5.0)


If you want to change the `weightMissing` and `valueMissing`, you can use `weightMissing` and `valueMissing` methods:
```scala
val aggregationWithMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(10.0).valueMissing(5.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val aggregationWithMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(10.0).valueMissing(5.0)
val aggregationWithValueAndWeightMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(10.0).valueMissing(5.0)

* @param valueField
* the type-safe field for value for which weighted avg aggregation will be executed
* @param weightField
* the type-safe field for weight for which weighted avg aggregation will be executed
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this:

Suggested change
* the type-safe field for weight for which weighted avg aggregation will be executed
* the type-safe field that represents weight for which weighted avg aggregation will be executed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Something like this for valueField also.

self.copy(weightMissing = Some(value))

private[elasticsearch] def toJson: Json = {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Omit empty line.

)))
)
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Omit empty line.

Comment on lines 95 to 96
case str if str.contains("weighted_avg#") =>
WeightedAvgAggregationResponse.decoder.decodeJson(data.toString).map(field.split("#")(1) -> _)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put this first and remove && !str.contains("weighted_avg#") from the first case.

weightedAvgAggregation("aggregation", TestDocument.stringField, TestDocument.intField).valueMissing(2.0)
val aggregationWithWeightMissing =
weightedAvgAggregation("aggregation", TestDocument.stringField, TestDocument.intField).weightMissing(3.0)
val aggregationWithBothMissing = weightedAvgAggregation(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val aggregationWithBothMissing = weightedAvgAggregation(
val aggregationWithValueAndWeightMissing = weightedAvgAggregation(

weightedAvgAggregation("aggregation", TestDocument.stringField, TestDocument.intField).valueMissing(2.0)
val aggregationWithWeightMissing =
weightedAvgAggregation("aggregation", TestDocument.stringField, TestDocument.intField).weightMissing(3.0)
val aggregationWithBothMissing = weightedAvgAggregation(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same.

@vanjaftn vanjaftn force-pushed the task-support-weighted-avg-aggregation branch 2 times, most recently from 384cbac to 4237565 Compare November 10, 2023 12:28
@vanjaftn vanjaftn changed the title (dsl): Support Weight Avg aggregation (dsl): Support Weight Avg aggregation Nov 10, 2023

If you want to add aggregation (on the same level), you can use `withAgg` method:
```scala
val multipleAggregations: MultipleAggregations = weightedAvgAggregation(name = "weightedAvgAggregation", valueField = Document.intField, weightField = Document.doubleField).withAgg(weightedAvgAggregation(name = "weightedAvgAggregation2", valueField = Document.doubleField, weightField = Document.intField))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val multipleAggregations: MultipleAggregations = weightedAvgAggregation(name = "weightedAvgAggregation", valueField = Document.intField, weightField = Document.doubleField).withAgg(weightedAvgAggregation(name = "weightedAvgAggregation2", valueField = Document.doubleField, weightField = Document.intField))
val multipleAggregations: MultipleAggregations = weightedAvgAggregation(name = "weightedAvgAggregation1", valueField = Document.intField, weightField = Document.doubleField).withAgg(weightedAvgAggregation(name = "weightedAvgAggregation2", valueField = Document.doubleField, weightField = Document.intField))


If you want to change the `weightMissing` and `valueMissing`, you can use `weightMissing` and `valueMissing` methods:
```scala
val aggregationWithValueAndWeightMissing: WeightedAvgAggregation = weightedAvgAggregation(name = "weightedAvgAggregation", field = Document.intField).weightMissing(10.0).valueMissing(5.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put valueMissing first.

@vanjaftn vanjaftn force-pushed the task-support-weighted-avg-aggregation branch from 8c2d3d2 to c76e95f Compare November 13, 2023 09:14
@dbulaja98 dbulaja98 merged commit 8f9990c into lambdaworks:main Nov 13, 2023
13 checks passed
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.

2 participants