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

Incorrect search directory docstring of qta.load_font() with directory=None #195

Closed
kumattau opened this issue Dec 18, 2021 · 4 comments · Fixed by #198
Closed

Incorrect search directory docstring of qta.load_font() with directory=None #195

kumattau opened this issue Dec 18, 2021 · 4 comments · Fixed by #198
Assignees
Labels
Milestone

Comments

@kumattau
Copy link
Contributor

https://qtawesomedocs.readthedocs.io/en/latest/_generate/qtawesome.load_font.html says,

qtawesome.load_font(prefix, ttf_filename, charmap_filename, directory=None)
Loads a font file and the associated charmap.

If directory is None, the files will be looked for in ./fonts/.

but qta.load_icon() with directory=None searches a font in "qtawesome/fonts", not current "./fonts/".
To avoid the problem and search a font in "./fonts/", users must set directory to "./fonts/" explicitly.

# -*- coding: utf-8 -*-
import sys
from qtpy import QtWidgets, QtCore
import qtawesome as qta

class AwesomeExample(QtWidgets.QDialog):

    def __init__(self):
        super().__init__()
        # Failed if directory=None
        qta.load_font("myicons", "myicons.ttf", "myicons-charmap.json")

        # Succeeds if directory is explicitly set to "./fonts/"
        # qta.load_font("myicons", "myicons.ttf", "myicons-charmap.json", directory="./fonts/")

        icon = qta.icon("myicons.verified-filled")
        button = QtWidgets.QPushButton(icon, "myicons")

        grid = QtWidgets.QGridLayout()
        grid.addWidget(button, 0, 0)
        self.setLayout(grid)
        self.setWindowTitle('Awesome')
        self.show()


def main():
    app = QtWidgets.QApplication(sys.argv)
    QtCore.QTimer.singleShot(10000, app.exit)
    _ = AwesomeExample()
    sys.exit(app.exec_())


if __name__ == '__main__':
    main()
$ python test.py
Traceback (most recent call last):
  File "test.py", line 33, in <module>
    main()
  File "test.py", line 28, in main
    _ = AwesomeExample()
  File "test.py", line 10, in __init__
    qta.load_font("myicons", "myicons.ttf", "myicons-charmap.json")
  File "/home/.local/lib/python3.8/site-packages/qtawesome/__init__.py", line 216, in load_font
    return _instance().load_font(prefix, ttf_filename, charmap_filename, directory)
  File "/home/.local/lib/python3.8/site-packages/qtawesome/iconic_font.py", line 285, in load_font
    with open(os.path.join(directory, ttf_filename), 'rb') as font_data:
FileNotFoundError: [Errno 2] No such file or directory: '/home/.local/lib/python3.8/site-packages/qtawesome/fonts/myicons.ttf'
@kumattau
Copy link
Contributor Author

We may need to consider carefully whether to fix the problem
because it is possible that there are applications depending on the incorrect behavior of directory=None.

There is qta.load_font('spyder', 'spyder.ttf', 'spyder-charmap.json') in the document,
but spyder sets directory argument explicitly.
So spyder does not depends on the incorrect behavior of directory=None at least.

https://github.com/spyder-ide/spyder/blob/7418676022a5ddaedaff562cbff822410856fc99/spyder/utils/icon_manager.py#L376-L377

@dalthviz
Copy link
Member

Thank you for the feedback @kumattau ! Seems like there is an inconsistency between the current functionality implemented and the docs. I think we need to change the docstring to say something along the lines of:

If directory is None, the files will be looked for in the QtAwesome `fonts` directory.

What do you think @kumattau ?

@dalthviz dalthviz added this to the v1.2.0 milestone Dec 20, 2021
@kumattau
Copy link
Contributor Author

@dalthviz

I think it is better to change docs keeping current API behaivor for backward compatibility.

@dalthviz dalthviz changed the title Incorrect search directory of qta.load_icon() with directory=None Incorrect search directory docstring of qta.load_font() with directory=None Dec 21, 2021
@dalthviz dalthviz self-assigned this Dec 21, 2021
@kumattau
Copy link
Contributor Author

Additionally, I think 2 contrasted examples (with directory and without directory) are needed for users to understand the usage like as follows:

# cp myicon.ttf myicon-charmap.json /path/to/lib/python/site-packages/qtawesome/fonts/

qta.load_font('myicon', 'myicon.ttf', 'myicon-charmap.json')
# cp myicon.ttf myicon-charmap.json /path/to/myproject

qta.load_font('myicon', 'myicon.ttf', 'myicon-charmap.json', directory='/path/to/myproject')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants