Add deprecation warning on get_counts

[jameslamb]

Overview

In this PR, I'd like to propose deprecating get_counts. This function is not really central to what uptasticsearch does and will require extra administrative burden to test. As a reminder, get_counts basically generates queries to check the counts of different values on a field within a period of time.

This is outside of our mission for a couple of reasons:

• assumes that you have time-series data
• writes query DSL inside the package
• only useful for a very specific type of EDA

IMHO we should remove this function from the package.

Deprecation Strategy

I propose accepting this PR so this warning goes out with the next release to CRAN (which we will attempt whenever #66 is building and gets merged).

After that release gets to CRAN, I'll submit a PR straight-up removing this function from uptasticsearch.

We will wait at least one month to do the next release, giving people time to uncover this deprecation warning.

IF any interest is shown in the function (via people creating issues), we can discuss keeping it.

[codecov-io]

Codecov Report

Merging #69 into master will decrease coverage by 0.74%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   68.43%   67.68%   -0.75%     
==========================================
  Files           4        4              
  Lines         548      554       +6     
==========================================
  Hits          375      375              
- Misses        173      179       +6

Impacted Files	Coverage Δ
R/elasticsearch_eda_funs.R	36.03% <0%> (-2.06%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff68525...2873a4c. Read the comment docs.

[austin3dickey]

Perhaps change this to @note? That may make more sense than "References"

[jameslamb]

good call

[austin3dickey]

I agree with this deprecation strategy. This function makes too many assumptions

[jameslamb]

@austin3dickey should we deprecate parse_date_time too (with the same strategy)? That functionality is also not specific to Elasticsearch

[austin3dickey]

I don't think so. That's a utility that parses data you get from Elasticsearch (timestamps of the form "2016-06-28T08:53:07Z") that's not replicable in base R (I don't think you can parse military time zone abbreviations) so it seems to me that it belongs

[jataggart]

I like the deprecation strategy, but is there any way or need to make it more obvious? Maybe call out the deprecations in the README or NEWS documents...

[jameslamb]

@jataggart good call, will add to the NEWS.md

@austin3dickey ok sure. I think there's a difference between "this function should exist somewhere in the R ecosystem" and "this function belongs in uptasticsearch".

That date format is not ES specific...it's called ISO 8601 and it's used in other places where a mixed date + time + TZ string representation is required.

For now, I think the message is "we all feel fine about deprecating get_counts but parse_date_time is more contentious". So I won't bundle it into this PR. Also it's not exposing us to the ES query DSL so I think it's a lower priority to figure out

[jameslamb]

Got it building! All of the failed builds were from transient Travis issues. I just kept clicking rebuild until they resolved themselves