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

PR: Fix error edge line in python2.7 #4155

Merged
merged 4 commits into from
Feb 17, 2017

Conversation

rlaverde
Copy link
Member

Fixes: #4153

@rlaverde rlaverde added this to the v4.0beta1 milestone Feb 16, 2017
@rlaverde rlaverde self-assigned this Feb 16, 2017
@rlaverde rlaverde requested a review from ccordoba12 February 16, 2017 22:55
@goanpeca
Copy link
Member

@rlaverde now that you did this.

Could we add a test to find with a regex or something

isinstance(.*,.*str) ?

@rlaverde
Copy link
Member Author

Could we add a test to find with a regex or something

I added it in the last commit, what do you think?

@goanpeca goanpeca merged commit 780d93d into spyder-ide:master Feb 17, 2017
@rlaverde rlaverde deleted the fix/edge-line-py2.7 branch February 17, 2017 21:03
@@ -94,7 +94,7 @@ def get(self, name_or_klass):
:param name_or_klass: Name or class of the panel to retrieve.
:return: The specified panel instance.
"""
if not isinstance(name_or_klass, str):
if not is_text_string(name_or_klass, str):
Copy link
Member

Choose a reason for hiding this comment

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

@rlaverde, this call is wrong because is_text_string only receives one argument.

@@ -76,7 +76,7 @@ def start(self):
data = page.read()

# Needed step for python3 compatibility
if not isinstance(data, str):
if not is_text_string(data, str):
Copy link
Member

Choose a reason for hiding this comment

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

This is also wrong, and it's what's making our tests to time out in master because checking if there's a new Spyder version is failing now.

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.

3 participants