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

feat: Added dataset_name and dataset_description params to export_notebook #60

Merged
merged 3 commits into from
Aug 7, 2023
Merged

feat: Added dataset_name and dataset_description params to export_notebook #60

merged 3 commits into from
Aug 7, 2023

Conversation

CBROWN-ONS
Copy link
Contributor

Added the ability to pass the dataset_name and dataset_description to edvart.ReportBase.export_notebook. Uses the parameters in edvart.ReportBase._generate_notebook to write the dataset name and description to the notebook.
Closes #59

@mbelak-dtml mbelak-dtml self-requested a review August 7, 2023 07:34
Copy link
Collaborator

@mbelak-dtml mbelak-dtml left a comment

Choose a reason for hiding this comment

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

Hi @CBROWN-ONS. Thanks for the contribution!
Just one minor thing: Please apply code style as described in the Contributing guideline.

@CBROWN-ONS
Copy link
Contributor Author

Sorry about that @mbelak-dtml. Missed it as I was just trying to set up this account on my personal machine. The code should be properly formatted now. It also made some changes to your imports in several different files. I've left these changes in the PR as I assume that's something you'd eventually change (as it is in the contributing guidelines).

@CBROWN-ONS
Copy link
Contributor Author

@mbelak-dtml It seems to have failed again after re-organising the imports. These were re-organised by the pre-commit style tests in the contributing guidelines.

@mbelak-dtml
Copy link
Collaborator

@mbelak-dtml It seems to have failed again after re-organising the imports. These were re-organised by the pre-commit style tests in the contributing guidelines.

@CBROWN-ONS I see what's wrong. There is a discrepancy between the contributing and the CI. The correct command for sorting imports is:
poetry run isort --line-length 100 --profile black edvart/ tests/
(the --line-length is missing in the contributing guideline). I opened a PR #61 fixing the contributing guideline. Could you please re-format the code using this command? After that, we can merge this PR.
Sorry about the confusion.

@mbelak-dtml mbelak-dtml self-requested a review August 7, 2023 11:31
@CBROWN-ONS
Copy link
Contributor Author

Hi @mbelak-dtml. Imports should be fixed now

@mbelak-dtml mbelak-dtml added this pull request to the merge queue Aug 7, 2023
Merged via the queue into datamole-ai:main with commit 6e411ad Aug 7, 2023
@CBROWN-ONS CBROWN-ONS deleted the 59-add-export_notebook-params branch August 8, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add parameters dataset_name & dataset_description to edvart.ReportBase.export_notebook
2 participants