-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Remove nonfunctional Axes3D.set_frame_on and get_frame_on methods. #25648
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to keep these functions and raise NotImplementedError
, because by removing we would inherit the functionality from Axes, which does not work for 3D.
I think we need to keep these functions and raise
NotImplementedError
, because by removing we would inherit the functionality from Axes, which does not work for 3D.
Ok. The python docs state that NotImplementedError
should not be used to indicate that an operator or method is not meant to be supported at all, but rather if the method is still being developed etc. Do you still want to go ahead with it? Do you have any more plans for this, like getting rid of the frame_on concept entirely?
From the note in the docs, it looks like the recommended approach is to set the method to None
.
https://docs.python.org/3/library/exceptions.html#NotImplementedError
Setting to none (plus leaving a comment there) seems the right approach. Thanks for checking the meaning of NotImplementedError.
My understanding of the guidance is to set the actual method to None
, rather than the return value. I couldn’t find any examples by googling, but here is one I happen to know about in another project, where we wanted to disable the __iter__
method.
Sorry for not seeing this until now. Good that things were clarified in the meanwhile. I think a note in doc/api/next_api_changes/removals/ would be nice. Basically saying that they were not working and therefore removed. If nothing else we can point at it, in case someone reports an error later. Other than that, is should be good to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timhoffm can you please have a new look?
PR Summary
Fixes #24689. Remove
Axes3D.set_frame_on
andget_frame_on
from the codebase and from the link in the documentation.