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

Improves Extensions.TryConvert to avoid test by exception #2109

Merged
merged 1 commit into from
Jun 12, 2018

Conversation

AlexCatarino
Copy link
Member

Description

Instead of letting PyObject.AsManagedObject throw an exception because the types do not match, we check whether the target type is assignable from the python object type.

Related Issue

This PR fixes PR #2102 that caused a bug in Symbol[] convertion.
Closes #2091 and fixes #2102

Requires Documentation Change

No.

How Has This Been Tested?

Unit tests were added to test all changes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Non-functional change (xml comments/documentation/etc)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description or feature-<issue#>-<description>

@AlexCatarino AlexCatarino requested a review from mchandschuh June 12, 2018 21:24
Copy link
Contributor

@mchandschuh mchandschuh left a comment

Choose a reason for hiding this comment

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

Great stuff! Thanks for the fix and the extra tests to cover these cases!

Instead of letting `PyObject.AsManagedObject` throw an exception because the types do not match, we check whether the target type is assignable from the python object type.

This PR fixes PR #2102 that caused a bug in Symbol[] convertion.

Unit tests were added to test all changes.
@AlexCatarino AlexCatarino force-pushed the bug-2091-indicator-register-throws branch from 5b0c197 to a4233c7 Compare June 12, 2018 21:39
@mchandschuh mchandschuh merged commit 1900e1d into master Jun 12, 2018
@AlexCatarino AlexCatarino deleted the bug-2091-indicator-register-throws branch June 12, 2018 22:23
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.

Indicator consolidator throws error in python algorithm
2 participants