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

fix: Add Column Header to the exported CSV from query command #1313

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

saurabh0402
Copy link
Contributor

Issue

#1285

The CSV exported from the mf query <metric> --csv <csv-file-name> command does not contain the column header. The CSV thus does not make too much sense.

Fix

  • The code here is writing data to the CSV file. The column names aren't getting added to the file.
  • With this PR, we are adding the column names returned from df.column_names to the CSV to fix the issue.

The CSV file generated using `mf query --csv <csv-file-name>` does not
contain the header. This commit updates the function to include the
column header as well
@cla-bot cla-bot bot added the cla:yes label Jul 2, 2024
Copy link
Contributor

@tlento tlento left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @saurabh0402 ! Apologies for the delay, I was on vacation and most of the team relies on reviewer tags.

Could you add a changelog entry to this PR? See the contributing guide for instructions. That'll also ensure you get properly credited in our release notes.

Thank you!

@saurabh0402
Copy link
Contributor Author

Hey @tlento, thanks for reviewing this. I have added the changelog, please take a look now.

@saurabh0402 saurabh0402 requested a review from tlento July 16, 2024 03:33
Copy link
Contributor

@tlento tlento left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks so much @saurabh0402 !

@tlento tlento merged commit ab20927 into dbt-labs:main Jul 16, 2024
9 checks passed
@saurabh0402
Copy link
Contributor Author

saurabh0402 commented Jul 18, 2024

Hey @tlento, when can we expect this to be released?
We just started using metricflow and are facing issues because of this

@tlento
Copy link
Contributor

tlento commented Jul 18, 2024

End of next week, maybe? I need to fix up some deployment issues to allow for a backport and patch release on dbt-metricflow, and that takes some time to test.

@saurabh0402
Copy link
Contributor Author

Thanks a lot.

tlento pushed a commit that referenced this pull request Jul 26, 2024
# Issue
#1285

The CSV exported from the `mf query <metric> --csv <csv-file-name>`
command does not contain the column header. The CSV thus does not make
too much sense.

# Fix
- The code
[here](https://github.com/dbt-labs/metricflow/blob/main/dbt-metricflow/dbt_metricflow/cli/main.py#L344)
is writing data to the CSV file. The column names aren't getting added
to the file.
- With this PR, we are adding the column names returned from
`df.column_names` to the CSV to fix the issue.
tlento added a commit that referenced this pull request Jul 27, 2024
…1354)

This commit is a backport of PR #1313 to dbt-metricflow 0.7.x

This will allow us to deploy @saurabh0402's fix for the following issue:

#1285

Note - efficacy was tested via the CLI by running hatch build and doing
a pip install
of the build package into a clean VM, and running `mf query` with the
`--csv` flag
enabled.

Co-authored-by: Saurabh Kumar <[email protected]>
@tlento
Copy link
Contributor

tlento commented Jul 31, 2024

@saurabh0402
Copy link
Contributor Author

Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants