Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix #39 - Refactor 04_portfolio_trade.ipynb notebook #44

Merged
merged 4 commits into from
Aug 16, 2019
Merged

Fix #39 - Refactor 04_portfolio_trade.ipynb notebook #44

merged 4 commits into from
Aug 16, 2019

Conversation

miguelusque
Copy link
Contributor

04_portfolio_trade notebook refacturing.

Any comments are welcomed!

@miguelusque miguelusque requested a review from yidong72 August 15, 2019 19:50
Copy link
Collaborator

@yidong72 yidong72 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 editing this notebook. It looks simpler and cleaner than before. I leave a few comments that need to be addressed.

Copy link
Contributor Author

@miguelusque miguelusque left a comment

Choose a reason for hiding this comment

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

Hi Yi,

I have completed everything but this one:

we want to point out the user needs to set "self.delayed_process = True" to enable this dask computation graph integration.

Could you please write the complete sentence? I cannot see either where that comment should be.

Thanks!

@miguelusque miguelusque requested a review from yidong72 August 16, 2019 16:44
@yidong72
Copy link
Collaborator

added my comments there

@miguelusque
Copy link
Contributor Author

Submitted a new commit with the latest changes requested.

Copy link
Collaborator

@yidong72 yidong72 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

@yidong72 yidong72 merged commit eee980b into NVIDIA:develop Aug 16, 2019
@avolkov1 avolkov1 mentioned this pull request Aug 16, 2019
@miguelusque miguelusque deleted the fix_39 branch August 16, 2019 20:06
@miguelusque miguelusque self-assigned this Aug 19, 2019
@miguelusque miguelusque changed the title Fix 39 - Refactor 04_portfolio_trade.ipynb notebook Fix #39 - Refactor 04_portfolio_trade.ipynb notebook Aug 20, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants