Skip to content

Conversation

@CaptainKanuk
Copy link
Contributor

The initialization of perf_tracker had been moved from init
in TradingAlgorithm to _create_generator. This caused perf_tracker
to not be ready when portfolio requested it. portfolio was consequently
not ready for access in init. portfolio can now be accessed in init
again, assuming valid sim_params are passed. Otherwise it will be
available in handle_data().

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3fe688c on moved-perf-tracker-assignment into eae41b8 on master.

@twiecki
Copy link
Contributor

twiecki commented Jul 17, 2014

Looks good. Could use a unittest so that the bug doesn't resurface.

@CaptainKanuk
Copy link
Contributor Author

Yep, first on my list for today.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 79506c5 on moved-perf-tracker-assignment into e139294 on master.

@CaptainKanuk
Copy link
Contributor Author

Should be ready to go.

@twiecki
Copy link
Contributor

twiecki commented Jul 17, 2014

Looks good. Have you confirmed that the test fails without the fix?

@CaptainKanuk
Copy link
Contributor Author

Yup it fails w/o the change.

@twiecki
Copy link
Contributor

twiecki commented Jul 17, 2014

Cool, thanks for checking. /signoff

@CaptainKanuk
Copy link
Contributor Author

I just realized, might it be better to remove the else statement and always assign it? sim_params are going to be initialized at that point in the code anyway.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) when pulling 96ceaa2 on moved-perf-tracker-assignment into e139294 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 96ceaa2 on moved-perf-tracker-assignment into e139294 on master.

@CaptainKanuk CaptainKanuk assigned twiecki and unassigned ssanderson Jul 17, 2014
@twiecki
Copy link
Contributor

twiecki commented Jul 21, 2014

@CaptainKanuk Yes, that's a good point. /sign-off

The initialization of perf_tracker had been moved from __init__
in TradingAlgorithm to _create_generator. This caused perf_tracker
to not be ready when portfolio requested it. portfolio was consequently
not ready for access in init. portfolio can now be accessed in init
again, assuming valid sim_params are passed. Otherwise it will be
available in handle_data() after _create_generator() is called.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 97c88c3 on moved-perf-tracker-assignment into 0ff6a1c on master.

CaptainKanuk added a commit that referenced this pull request Jul 21, 2014
BUG: Put initialization of perf_tracker back in __init__
@CaptainKanuk CaptainKanuk merged commit 62c9b96 into master Jul 21, 2014
@twiecki twiecki deleted the moved-perf-tracker-assignment branch July 29, 2014 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants