-
Notifications
You must be signed in to change notification settings - Fork 524
Line protocol leading comma #694
Line protocol leading comma #694
Conversation
… the first value column is Null valued
@d3banjan thank you for contributing! can you fix up the broken tests please? |
Yes. I already took a look at the failed tests. The fix b4ceab9 passes the tests and is minimal! Umm... do you think the refactor commit 366e771 is an acceptable direction to work on? I kept wondering if this is indeed duplicated code or if I was missing something. |
I abandoned the idea of the refactor for now. |
@d3banjan would you mind adding a test to ensure that this situation is properly addressed? |
…invalid line protocol
@xginn8 Thanks for your patience, I have added a test with all possible cases considered and tried to make it as reproducible and deterministic as I could using pandas. |
@aviau : Can this be merged ? |
Sure, looks good. |
Environment:
pandas==0.24.1
-e git+ssh://git@github.com/KuguHome/influxdb-python.git@afcfd25b21523d84a7d1088eff2abb4d08de7647#egg=influxdb
which mirrors the current master branch of influxdb-pythonpython=3.6.7
Reproduction of bug:
bug-reproduce.py
Outlook:
Although tags and fields are handled the same way, the presence of a leading comma works for the tags. The following code is where the values are handled :
influxdb-python/influxdb/_dataframe_client.py
Lines 381 to 394 in afcfd25
Notice the difference to how tags are handled, in that the None columns are filtered out by value before conversion to line protocol. The values, however, are handled by the column index i.e
field_df[field_df.columns[1:]] = ',' + field_df[ field_df.columns[1:]]
influxdb-python/influxdb/_dataframe_client.py
Lines 366 to 369 in afcfd25
Fix and refactor:
,
fromfields
where appropriate.KuguHome@b4ceab9
Output:
__lineify_tag_field_df
, because DRY.The leading
,
needs to be taken out from thetags
output and needs to be put back when constructing the line again.KuguHome@366e771
Output:
Pull request at #694