python / cpython

The Python programming language
https://www.python.org
Other
63.52k stars 30.43k forks source link

tkinter: improve font.actual doc #86017

Open 919475b3-36a2-4cd7-997c-9c38f05f93c7 opened 4 years ago

919475b3-36a2-4cd7-997c-9c38f05f93c7 commented 4 years ago
BPO 41851
Nosy @taleinat, @serhiy-storchaka, @E-Paine
PRs
  • python/cpython#22396
  • python/cpython#22434
  • 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 = ['type-feature', 'expert-tkinter', '3.10', 'docs'] title = 'tkinter: add font equal methods' updated_at = user = 'https://github.com/E-Paine' ``` bugs.python.org fields: ```python activity = actor = 'taleinat' assignee = 'docs@python' closed = False closed_date = None closer = None components = ['Documentation', 'Tkinter'] creation = creator = 'epaine' dependencies = [] files = [] hgrepos = [] issue_num = 41851 keywords = ['patch'] message_count = 7.0 messages = ['377446', '377528', '377529', '377530', '377588', '377589', '377602'] nosy_count = 5.0 nosy_names = ['taleinat', 'gpolo', 'docs@python', 'serhiy.storchaka', 'epaine'] pr_nums = ['22396', '22434'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue41851' versions = ['Python 3.10'] ```

    919475b3-36a2-4cd7-997c-9c38f05f93c7 commented 4 years ago

    I think it would be helpful to add these methods to the tkinter font class.

    Starting with the equal method, I have found it very useful to be able to compare tkinter fonts based on their value rather than just name. I used this, for example, in bpo-28694 to ensure the -font attribute had been changed correctly (though there I compared each of the attributes explicitly rather than using .actual() as I have in this patch).

    The neg method, I think, would be very useful to allow easy conversion between points and pixels. For example:

    points = Font(size=16)
    pixels = -points
    taleinat commented 4 years ago

    Why would negating the font size convert it from points to pixels?? It seems to be coming straight from Tk; is this documented anywhere?

    I'd greatly prefer that we use a properly named property instead, e.g. .pixel_size.

    taleinat commented 4 years ago

    As for an equal() method, I think that adding equal() which is different than the existing __eq__() would be a source of confusion. I also think using it wouldn't add anything, and would be less clear, compared to font1.actual() == font2.actual().

    I'm definitely not happy with the existing __eq() though, and think it would be worth discussing changing __eq() to use self.actual() == other.actual() rather than just comparing names. That should be a separate issue, though - you're welcome to create one!

    919475b3-36a2-4cd7-997c-9c38f05f93c7 commented 4 years ago

    Why would negating the font size convert it from points to pixels??

    Oh... I apologise: I clearly didn't think this through. It doesn't "convert" in any sense, it just negates the font size so 16 points becomes 16 pixels. Therefore, it is practically useless (besides, a new font object would not help the user a converted value anyway) - I therefore do not wish to proceed this this part of the issue (I have removed it from the title and will remove it from the PR).

    I also think using [.equal] wouldn't add anything

    I disagree and will stand by this one! While the user could easily write it themselves, I think it would be helpful to have a convenience method to check if two fonts represent the same thing that is displayed to the user. When first creating this issue, I didn't want to touch __eq__ for two reasons:

    1. I didn't know how much code changing this would break (it is very hard to check for code usage of dunder methods - if you know any ways please let me know!)
    2. Checking to see if the fonts are the same Tk object is also very helpful (in other, granted fewer, contexts)

    For example, something like this could be used in IDLE so that if the user sets their font back to the equivalent of the default ("TkFixedFont"), the value isn't kept in the user's local config (similar to how all the other options do).

    Therefore, while I would like this somehow added, I agree that adding an equal method that does not do the same thing as == is confusing (you just have to look at Java and all the confusion over using == on strings - a 'gotcha' for every new Java learner). However, as I said above, I am reluctant to touch eq. Any ideas?

    taleinat commented 4 years ago

    > I also think using [.equal] wouldn't add anything

    I disagree and will stand by this one!

    Please do note that the full quote was "I also think using it wouldn't add anything, and would be less clear, compared to font1.actual() == font2.actual()".

    I think it would be helpful to have a convenience method to check if two fonts represent the same thing that is displayed to the user.

    More convenient than font1.actual() == font2.actual() ?

    Since this will be used rather rarely, IMO the more significant problem is discoverability - I've done a *lot* of work with Tkinter and had never heard of the .actual() method. However, adding another comparison method won't help with that by itself.

    I am reluctant to touch __eq__. Any ideas?

    You wrote that "Checking to see if the fonts are the same Tk object is also very helpful (in other, granted fewer, contexts)": Could you elaborate on that?

    I still think we should consider overriding __eq. But I need to check how consistent Tktinter is regarding __eq checking whether two objects have the same name.

    [goes to check the docs, code, and do some interactive investigation...]

    Okay, so the Font class's .name attribute is just a name the user gives to a certain font configuration, which is supposed to be unique. Our docs are pretty clear about this:

    "The Font class represents a named font. Font instances are given unique names and can be specified by their family, size, and style configuration. Named fonts are Tk's method of creating and identifying fonts as a single object, rather than specifying a font by its attributes with each occurrence." So in this case, tkinter is letting a particularly confusing Tk detail show through.

    Note that providing a name isn't required, and how confusing the result is when not providing font names:

    >>> Font(family='DejaVu Sans', size=12) == Font(family='DejaVu Sans', size=12)
    False

    Also, if you happen to try to create a font with a name which has already been used:

    >>> Font(name='default font', family='DejaVu Sans', size=12)
    <tkinter.font.Font object at 0x7f96ebbf5e60>
    >>> Font(name='default font', family='DejaVu Sans', size=12)
    Traceback [...]
    _tkinter.TclError: named font "default font" already exists

    One can avoid this error by passing exists=True, but using that when the font doesn't exist raises an exception:

    >>> Font(name='other font', family='DejaVu Sans', size=10, exists=True)
    Traceback [...]
    _tkinter.TclError: named font other font does not already exist

    My point is: Using named fonts in tkinter seems incredibly annoying. Using fonts without names seems to work better. But without using names, font comparison is meaningless.

    It seems to me that we could change eq to:

    1. Keep the current behavior if both font objects have explicitly set names or if their automatically generated names are equal (i.e. self is other).
    2. Otherwise, compare the the results of calling their .actual() methods.

    This would require adding a simple internal flag in __init to remember whether the name was set explicitly, since __init sets a unique name if not provided with one.

    P.S. The "name" constructor parameter is confusing! Here even Terry J. Reedy got it wrong in IDLE font configuration dialog's code: https://github.com/python/cpython/blob/d9ab95ff1fe81efdf70e545d536d9f6927d1ba81/Lib/idlelib/config.py#L745)

    taleinat commented 4 years ago

    P.P.S. Re-reading that piece of code for IDLE's font config dialog, that actually looks to be intentional and correct.

    serhiy-storchaka commented 4 years ago

    I concur with Tal that negating size would be useless feature, and that we do not need an alternate equal() method. When we want to test that two strings are equal ignoring case we do not call a special method equalignorecase(), we just test that s1.casefold() == s2.casefold().

    But I disagree that Font.__eq__() should be changed. The Font object is just a reference to the Tk font as the Path object is a reference to a file on filesystem. When we compare two Path objects we do not compare file sizes, modification times, permission bits and content. We just compare two full names. If two Path objects has the same full name, they are equal. If they full names are different, they are different, even if the corresponding files has the same content. The same is with Font objects.