ropensci / elastic

R client for the Elasticsearch HTTP API
https://docs.ropensci.org/elastic
Other
244 stars 58 forks source link

fix JSON format for list input #174

Closed pieterprovoost closed 7 years ago

pieterprovoost commented 7 years ago

This fixes some issues I had with the JSON produced in make_bulk() for list input:

This may break things for other users, please evaluate.

codecov-io commented 7 years ago

Codecov Report

Merging #174 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #174   +/-   ##
=======================================
  Coverage   63.84%   63.84%           
=======================================
  Files          37       37           
  Lines        1300     1300           
=======================================
  Hits          830      830           
  Misses        470      470
Impacted Files Coverage Δ
R/docs_bulk_utils.R 77.94% <100%> (ø) :arrow_up:

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 5f2b557...dd563d1. Read the comment docs.

sckott commented 7 years ago

thanks @pieterprovoost will have a look and see if this will be okay, does make sense to set NA's to null for json

sckott commented 7 years ago

seems good, except noticed that it has the effect that your change leads to all fields being retained in ES with null's retained, while on master now ES just drops those fields that have NA's - i imagine meaning data in ES is smaller with version on master now.

i'll note that null values can't be searched https://www.elastic.co/guide/en/elasticsearch/reference/5.3/null-value.html

i'll add some tests for NA's in data.frame's, lists, and files