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

Minor fixes in exclusions.py. #239

Merged
merged 1 commit into from
Aug 11, 2015
Merged

Minor fixes in exclusions.py. #239

merged 1 commit into from
Aug 11, 2015

Conversation

ctk3b
Copy link
Member

@ctk3b ctk3b commented Aug 11, 2015

No description provided.

@mrshirts
Copy link
Contributor

What was the bug here?

@ctk3b
Copy link
Member Author

ctk3b commented Aug 11, 2015

No bug, mostly code style. __repr__ and __str__ should return, not print and isinstance is the preferred way to check types .

@mrshirts
Copy link
Contributor

Sure, as long as it runs correctly. It hasn't changed any of the tests?

On Tue, Aug 11, 2015 at 12:00 PM, Christoph Klein notifications@github.com
wrote:

No bug, mostly code style. repr and str should return, not print
and isinstance is the preferred way to check types .


Reply to this email directly or view it on GitHub
#239 (comment).

@ctk3b
Copy link
Member Author

ctk3b commented Aug 11, 2015

All the unit tests run locally for me. If this breaks something, we have more serious issues :)

@swails
Copy link

swails commented Aug 11, 2015

Actually, I think the __repr__ and __str__ were bugs. I don't think they're allowed to return None

In [3]: class SomeClass(object):
   ...:     def __repr__(self):
   ...:         print('Inside __repr__')
   ...:     def __str__(self):
   ...:         print('Inside __str__')
   ...:         

In [4]: x = SomeClass()

In [5]: x
Inside __repr__
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/iscratch/miniconda/2/envs/py27_1/lib/python2.7/site-packages/IPython/core/formatters.pyc in __call__(self, obj)
    688                 type_pprinters=self.type_printers,
    689                 deferred_pprinters=self.deferred_printers)
--> 690             printer.pretty(obj)
    691             printer.flush()
    692             return stream.getvalue()

/iscratch/miniconda/2/envs/py27_1/lib/python2.7/site-packages/IPython/lib/pretty.pyc in pretty(self, obj)
    405                             if callable(meth):
    406                                 return meth(obj, self, cycle)
--> 407             return _default_pprint(obj, self, cycle)
    408         finally:
    409             self.end_group()

/iscratch/miniconda/2/envs/py27_1/lib/python2.7/site-packages/IPython/lib/pretty.pyc in _default_pprint(obj, p, cycle)
    525     if _safe_getattr(klass, '__repr__', None) not in _baseclass_reprs:
    526         # A user-provided repr. Find newlines and replace them with p.break_()
--> 527         _repr_pprint(obj, p, cycle)
    528         return
    529     p.begin_group(1, '<')

/iscratch/miniconda/2/envs/py27_1/lib/python2.7/site-packages/IPython/lib/pretty.pyc in _repr_pprint(obj, p, cycle)
    707     """A pprint that just redirects to the normal repr function."""
    708     # Find newlines and replace them with p.break_()
--> 709     output = repr(obj)
    710     for idx,output_line in enumerate(output.splitlines()):
    711         if idx:

TypeError: __repr__ returned non-string (type NoneType)

As for the type checking, isinstance is a better choice, since it works with inheritance.

mrshirts added a commit that referenced this pull request Aug 11, 2015
Minor fixes in exclusions.py.
@mrshirts mrshirts merged commit 2a1e840 into shirtsgroup:develop Aug 11, 2015
@ctk3b ctk3b deleted the patch branch November 10, 2016 16:03
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.

None yet

3 participants