Skip to content

Support extended_bounds on Histogram #1094

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

Merged
merged 1 commit into from
Dec 9, 2014
Merged

Support extended_bounds on Histogram #1094

merged 1 commit into from
Dec 9, 2014

Conversation

ftoft
Copy link
Contributor

@ftoft ftoft commented Dec 6, 2014

I have added support for extended_bounds on histogram, i hope this is a welcome contribution.

@@ -56,6 +60,8 @@ public class HistogramAggregationDescriptor<T> : BucketAggregationBaseDescriptor

IDictionary<string, string> IHistogramAggregator.Order { get; set; }

IDictionary<string, object> IHistogramAggregator.ExtendedBounds { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer a strongly typed ExtendBounds object with Min and Max as properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! I just looked at how it was made in date histogram.

If i make a ExtendedBounds object, with two properties Min as Double and Max as Double, it's not posseible to reuse this object in DateHistogram, because it will be bounded to DateTime.

Maybe this is not a problem? What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Dang, i think I screwed up there :) I'll pull your PR as is and add a TODO to move both to a typed object in NEST 2.0. We can't change properties in minor releases do to strict semver requirements.

@Mpdreamz
Copy link
Member

Mpdreamz commented Dec 6, 2014

It sure is! thanks @ftoft 👍

I left one remark on the PR itself.

Mpdreamz added a commit that referenced this pull request Dec 9, 2014
Support extended_bounds on Histogram
@Mpdreamz Mpdreamz merged commit f2ba42b into elastic:develop Dec 9, 2014
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