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

allow NotImplementedError in formatters #4832

Merged
merged 3 commits into from Jan 24, 2014

Conversation

minrk
Copy link
Member

@minrk minrk commented Jan 20, 2014

without warning

also make the warnings regular Python warnings, so they can be suppressed.

closes #4792

@Carreau
Copy link
Member

Carreau commented Jan 20, 2014

Make sens. Would you like to add handeling for returning NotImplemented ?

adds FormatterWarning warning class, available for suppression
@minrk
Copy link
Member Author

minrk commented Jan 20, 2014

Would you like to add handling for returning NotImplemented?

No, returning NotImplemented is only for overriding comparison methods, not for general use.

@Carreau
Copy link
Member

Carreau commented Jan 20, 2014

No, returning NotImplemented is only for overriding comparison methods, not for general use.

Ok. I could argue that Python does not have "rich representation" and that the Python doc use the term "rich comparaison" (sic).

@jasongrout
Copy link
Member

@minrk: I'm curious: what sources suggest that NotImplemented is not for general use, especially in situations like rich comparison, when you want to signal that the system should try some other ways to accomplish a result?

@minrk
Copy link
Member Author

minrk commented Jan 20, 2014

@Carreau It is a similar case, sure. But we do not handle it in the way Python comparison handles NotImplemented, which is to proceed with dispatch as if that entry didn't exist.

@jasongrout Python docs. It doesn't say it's specifically to be avoided elsewhere, but it does describe the case for which it exists, and I have not seen it used for anything else.

@minrk
Copy link
Member Author

minrk commented Jan 20, 2014

@jasongrout I don't feel strongly, it just seems like a sufficiently rare pattern that we should only handle the more common one. If the NotImplemented singleton is more common outside of comparison methods than I thought, I have no problem using it.

@Carreau
Copy link
Member

Carreau commented Jan 20, 2014

I'm fine with not handeling NotImplemented, we can put that on dev meeting agenda.

@jasongrout
Copy link
Member

I don't feel strongly about it either, but I do feel like handling NotImplemented is a nice solution in the discussion on _ipython_display_ and the _repr_* framework.

@takluyver
Copy link
Member

I don't feel strongly about which we use, but I do think we should use just one of NotImplemented or NotImplementedError - allowing both just makes the API more confusing.

@minrk
Copy link
Member Author

minrk commented Jan 21, 2014

I'm coming around to NotImplemented in terms of basic design, which @jasongrout prefers, but there's one point where NotImplementedError has a big advantage: it's backward-compatible. Returning NotImplemented will raise a TypeError in IPython json serialization, whereas raising NotImplementedError will have the same behavior in 1.0 and 2.0. For that alone, if we pick just one, I think it should be NotImplementedError, even if it is the less elegant.

@ghost ghost assigned ellisonbg Jan 23, 2014
@takluyver
Copy link
Member

We discussed NotImplemented vs NotImplementedError in the dev meeting yesterday, and agreed to stick with NotImplementedError for the reason Min gives above.

@minrk, is there anything else you want to do before we merge this?

@minrk
Copy link
Member Author

minrk commented Jan 24, 2014

Nope, it should be all set.

takluyver added a commit that referenced this pull request Jan 24, 2014
allow NotImplementedError in formatters
@takluyver takluyver merged commit b823a5a into ipython:master Jan 24, 2014
@ghost
Copy link

ghost commented Jan 27, 2014

I'm noticing that executing a cell that raises such an error pops up a Formatter warning on first execution
but repeated execution of the same cell or the same code in a new cell does not produce a warning.

Is that intentional?

@minrk minrk deleted the formatter-not-implemented branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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.

repr_html exception warning on qtconsole with pandas #4745
5 participants