Open 919475b3-36a2-4cd7-997c-9c38f05f93c7 opened 4 years ago
For years, the Python docs for the tkinter.dnd module (and prior Tkdnd module) have said that it will become deprecated once it has been replaced by TkDND bindings (I can find it back in the Python 2.2 docs – https://docs.python.org/2.2/lib/node508.html). Despite this, I cannot find anywhere that the changes are actually proposed (have I missed something?!).
Attached is a git diff of a draft of proposed changes for the tkinter library, tkinter docs and Windows installer. The changes to the tkinter library are designed so that TkDND is not required where tcl/tk is installed, and in particular I check before any call is made to the library that it has been successfully loaded so as to give the user a more helpful error message.
I don’t feel it is ready to be a PR yet, unless that would be helpful, as I have not been able to extensively test on MacOS (I think I have sufficiently tested on Windows and Linux, though someone is bound to find a problem!). I also don’t feel it is ready for a PR because I would really appreciate if someone could take a serious look at the changes to the installer. I have got it working but I don’t really understand how the installer works and it is quite likely that the TkDND externals should have their own group and bindpath.
On a similar note to the Windows installer, how do we declare to the Linux package maintainers that TkDND is now an optional dependency of Python? I cannot find a file where optional dependencies are mentioned, so is there some mechanism to notify them of the changes?
I want to give credit where it is due and so it should be noted that new docs are very nearly just a “translation” of the TkDND man page and that the changes in the tkinter library are (loosely) based on the TkinterDND bindings.
I also have the relevant files for the cpython-source-deps and cpython-bin-deps repos, but would need someone with write permissions to create a ‘tkdnd’ branch in each of those repos before I create the PRs for them.
I am very new to contributing and so have a few questions on what is expected of me:
If people don’t generally want to add TkDND bindings to the tkinter library, I would instead propose that we remove the note that the tkinter.dnd module will be deprecated (but leave the note about it being experimental).
Easiest is to install 'blurb' and use that to write news entries. See https://docs.python.org/3/whatsnew/changelog.html#changelog for examples. The coredev who merges may changes whatever you propose.
I don't think that this needs a PEP, unless Serhiy decides otherwise. I don't think that there are very many details to thrash out.
Is there any reason for TkDND to be optional apart from the rest of Tkinter? If not, your installer changes are probably fine, but why not combine the two and just treat it as part of the Tcl/Tk build? (Primarily for Windows, I'd guess. Not sure how that'd work for macOS.)
Submitting a PR helps by running checks, and you can mark it as draft to prevent anyone from merging it. Though since it needs to pull down prebuilt binaries, it's not going to work until we sort out how to build the separate pieces.
Terry:
Thank you, I have updated my local version with a short descriptor and am relieved that I probably don't need to write a PEP as I am not known for my writing skills!
Steve:
Also thank you. I cannot think of a reason for it to be optional on Windows, however I think it would be best for it to remain a separate external (though, for obvious reasons, you should get the final say). There are two benefits, I think, that being a separate external brings:
Easier to update - I guess this depends on your experience of Tix (as an example), but for maintainability, I think it would be easier to update TkDND separate to updating tcl/tk.
Old Python builds don't included unneeded binaries - For older python versions, for which the changes to the tkinter library are not going to be backported (3.6 - 3.9), it seems pointless to package the TkDND binaries.
Easy to make optional in future - Less important, but if we change our mind about making TkDND optional (as an installer build flag, etc.), the change would be made much easier (whether that be during this issue/PR or as another issue).
I will go about creating a PR for this, though if I am feeling particularly incompetent, may need you to set it as a draft!
Ned:
What changes would be required to the MacOS installer to declare TkDND as an optional dependency (I really don't have a clue how MacOS works so would really appreciate some help)? I presume, based on the READMEs that it doesn't use the source- & bin-deps repos, but it also doesn't have a package manager like Linux distros to which to specify the optional dependencies.
Anyone!
Could the person responsible for the Linux builds please be added to the nosy, so that I can ask the question about notifying the package maintainers for each of the distros. I would also like to ask if TkDND could be declared as an optional dependency, as many distros do not have it in their main repos (taking Arch Linux as an example, the TkDND package is in the AUR, while tk is in the "extra" repo).
What changes would be required to the MacOS installer to declare TkDND ...
Let's get Windows settled first and then I will look at adding it to the macOS installer.
Regarding other platforms: we don't provide binaries for anything other than our own Windows and macOS installers. So I think there are a dew issues here:
What to do about third-party distributors of Python? (That includes macOS and Windows). The best we can do, probably, is to make sure the recommended dependency is mentioned in the What's New in 3.10 document. There may be some appropriate mailing lists or groups to discuss further; I vaguely recall there being one someone that was focused on distributors of Python. But there will be a *lot* of variability here since not all Python distributors also distribute their own version of Tcl/Tk for use with Python.
We would also want to make sure that our buildbot fleet is updated to include TkDND for those buildbots that attempt to do gui tests.
Addressing Ned's issues:
I have emailed round "Linux-sig" about adding an optional dependency and Guido recommended I put it on "Python-dev" instead (which I hope to do in the coming days). As for the buildbots, is this something I could help with, or does it just require someone with permission to remote-logon and change as necessary?
As a side-note, I currently looking at better ways of getting binding returns to TkDND as the current method (https://github.com/python/cpython/pull/20896/commits/9a27b147239c3326046ac439abccc07127b16f91#diff-3eeb1d6bdd82965287a553385c879693R1449) is not that clear ('dndrtn' does nothing - the set method simply returns the variable contents without trying to evaluate it).
As for the buildbots, is this something I could help with
@zach.ware is the best person to work with on any buildbot requirements. I've Nosyed him here.
For the record, I haven't yet tried to test TkDND with the various current macOS Tk on current versions of macOS; there are a lot of variables there and a lot of ways it may have issues. I hope to take a first look soon.
I have now marked the PR ready for review.
Terry:
Could you please take a quick look to double-check that the changes proposed are fully backwards-compatible? I have run the IDLE tests and have tried to pay particular attention to this problem when developing, but would appreciate a second opinion (to ensure we won't break existing tkinter scripts - such as IDLE).
Serhiy:
I would really appreciate your opinion on the patch, particularly the change to the _bind method. I am also concerned about the getlist_event method as it can split dropped text data (for example) but I don't know how to fix it (unless we give the user the raw data for them to fix it?).
I think I have fixed the problem with the 'data' attribute (and related getlist_event method) by exposing two event data attributes. One of these is a 'data_raw' attribute intended for use with text-based drops and the other is a 'data_split' intended for file drops (see the new docs for details).
My original idea was to expose the 'tkapp.splitlist' method and just give them the content of what is now the 'data_raw' attribute. However, I decided that should be avoided and so settled on the current implementation of two new attributes.
Thanks everyone for the effort you have already put into this and I know that there is a lot added in this PR but I would really appreciate some reviews to get it to a more commit-ready state (there is inevitably lots to be improved - I also apologise for saying 'appreciate' in almost every message :-).
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields: ```python assignee = None closed_at = None created_at =
labels = ['expert-tkinter', 'type-feature', 'library', '3.10']
title = 'tkinter: integrate TkDND support'
updated_at =
user = 'https://github.com/E-Paine'
```
bugs.python.org fields:
```python
activity =
actor = 'epaine'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Tkinter']
creation =
creator = 'epaine'
dependencies = []
files = ['49217']
hgrepos = []
issue_num = 40893
keywords = ['patch']
message_count = 9.0
messages = ['370852', '371445', '371554', '371577', '371593', '372110', '372111', '372421', '372705']
nosy_count = 7.0
nosy_names = ['terry.reedy', 'gpolo', 'ned.deily', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'epaine']
pr_nums = ['20896']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40893'
versions = ['Python 3.10']
```