dataiku / dss-plugin-neo4j

A DSS Plugin to interact with the Neo4j graph database
Apache License 2.0
7 stars 1 forks source link

Adding functionality to override the pandas default na values. #33

Closed SFuller4 closed 1 year ago

SFuller4 commented 1 year ago

This is directly solving this issue [https://github.com/dataiku/dss-plugin-neo4j/issues/29](Issue #29).

SFuller4 commented 1 year ago

Handling NaN Values

Usage Notes

SFuller4 commented 1 year ago

@StanislasGuinel I updated the request and resolved your comments. I've also added a comment that has the data for the documentation page. image

I also added the new parameters to the pytest file, although I was unable to perform the pytest locally.

Let me know if there are any other problems so I can help get this in place as soon as possible!

StanislasGuinel commented 1 year ago

Thanks for the comment for the documentation page !

Did you test that your feature work ? It still fails (TypeError: create_dataframe_iterator() got an unexpected keyword argument 'na_value')

StanislasGuinel commented 1 year ago

Also, there are many line diffs in this PR that shouldn't be there, I guess you used a formatter, but that makes the code review much harder. Could you revert all the useless diffs so that the review can only focus on the new feature code ?

SFuller4 commented 1 year ago

Sorry about that failure, I did run it on our instance, but after your message and checking, I had file discrepancies between what was on the instance and my local machine.

I've also reverted most of the black formatting changes. I was worried about that on the initial pull request, but figured it would be okay since it's generally required for open-source code to be formatted black or lint before pushing.

SFuller4 commented 1 year ago

Those changes are good with me! I went ahead and updated my previous comment for the documentation to use NaN instead of NULL as well, to stay consistent.

SFuller4 commented 1 year ago

Brought the Export Relationships json in line with Export Nodes json.

StanislasGuinel commented 1 year ago

@SFuller4 the new version of the plugin has been published !

SFuller4 commented 1 year ago

Thanks for the help @StanislasGuinel ! Everything updated and looks great from our side!