Skip to content
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

Add CSV export query results module #199

Merged
merged 19 commits into from Mar 20, 2023
Merged

Conversation

katarinasupe
Copy link
Contributor

@katarinasupe katarinasupe commented Mar 2, 2023

Description

I created csv_query procedure inside export_util module. Find arguments description in docstring.

Discussed with @Josipmrden:

  • Potentially use pandas instead of csv - what are the pros? - Use csv, pandas would be new dependency, and csv is not.
  • Leave config as a map or add stream as an argument. Another option is to return the stream and remove it from the arguments altogether. I vote for this. - The option to always return the stream is not good with big results, since it always has to write to stream, even if the user only wants to save in file. The config parameter does not make sense for MAGE modules, so we will go with stream as argument.
  • Probably edit the path inside the code to be /usr/lib/memgraph/query_modules instead of /var/lib/memgraph/internal_modules used for development. In json() procedure in this module, I left it to the user to provide the whole path and advised them to provide /usr/lib/memgraph/query_modules/something.json. This may be a better way to go. - We will go with path and handle potential errors

TODOs:

Pull request type

  • Algorithm/Module

######################################

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

  • Core algorithm/module implementation
  • Query module implementation
  • Unit tests
  • End-to-end tests
  • Code documentation
  • README short description
  • Documentation on memgraph/docs

######################################

@katarinasupe katarinasupe added lang: python Issue on Python codebase status: draft PR is in draft phase type: module labels Mar 2, 2023
@katarinasupe katarinasupe self-assigned this Mar 2, 2023
@Josipmrden
Copy link
Collaborator

As for Pandas, I think it makes it more easier to work with CSV files, but we can check what are the implications of adding another dependency into MAGE.

As for the second one, I'm fine with both.

  1. ok, makes sense

@antoniofilipovic antoniofilipovic added this to the v1.6.1 milestone Mar 3, 2023
@antoniofilipovic
Copy link
Collaborator

@katarinasupe what is benchmarking difference between csv and pandas? When doing writes. Is there any difference

@antoniofilipovic antoniofilipovic self-requested a review March 3, 2023 15:03
@katarinasupe
Copy link
Contributor Author

I did not benchmark it @antoniofilipovic. csv can be used out of the box, while pandas must be added as a new dependency to MAGE.

@katarinasupe katarinasupe added status: ready PR is ready for review and removed status: draft PR is in draft phase labels Mar 6, 2023
@katarinasupe katarinasupe marked this pull request as ready for review March 6, 2023 15:06
@katarinasupe
Copy link
Contributor Author

@Josipmrden, do you know what is happening with tests? Suddenly they decided to fail 😂

@antoniofilipovic antoniofilipovic added status: ship it PR approved and removed status: ready PR is ready for review labels Mar 15, 2023
@antoniofilipovic antoniofilipovic merged commit d46005f into main Mar 20, 2023
4 checks passed
@antoniofilipovic antoniofilipovic deleted the add-csv-export-query-module branch March 20, 2023 10:35
@antepusic antepusic mentioned this pull request Apr 5, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: python Issue on Python codebase status: ship it PR approved type: module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants