-
-
Notifications
You must be signed in to change notification settings - Fork 688
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
Split canvas.py into subpackage #3053
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.
We can remove the 2 "organisational" comments that you've flagged, but otherwise this looks like a good organizational cleanup to me.
On the subject of cleanups - if we're doing a big cleanup like this, then it might be worth finalizing the deprecations. The deprecation paths have been in place for over 18 months, before the 0.4.0 release; since we're about to bump to 0.5.0, we might as well finish that cleanup. I'll leave in your hands wWhether we clean up the canvas deprecations in this PR, or do a separate "Remove pre 0.4.0 deprecations" PR.
Since it would be a good idea to clean up other deprecations too, yeah, I think I'll do that in a separate PR. |
Question: should I also remove things that were marked as deprecated in the 0.4.0 release, or only those already marked before it? |
If the source tarball for 0.4.0 contains the code for the deprecation, the deprecation can be removed for the 0.5.0 release (which will likely be cut once I'm back in the office in the new year). |
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.
👍 Thanks for the cleanup!
canvas.py
is currently 1,747 lines long, and the actual Canvas widget doesn't start until 1,209. This PR splits it up into more manageably-sized files. The__init__.py
exports all of the names, so nothing should change as far as use from outside of Canvas.PR Checklist: