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

Allowing to pass font as file-like objects #202

Closed
wants to merge 10 commits into from
Closed

Allowing to pass font as file-like objects #202

wants to merge 10 commits into from

Conversation

Mibou
Copy link
Contributor

@Mibou Mibou commented Apr 22, 2013

This is the update to allow to pass fonts as file-like objects. Thank you wiredfool and aclark4life.

@wiredfool
Copy link
Member

A couple things I want to check:

  • The buf object has to be read only while the font is using it.
  • We're going to need tests on this one.
  • I'm not 100% sure on the interface changes. When we're reading images, the filename/file like object is passed in as the same parameter. I don't generally like variable type parameters, but it's consistent with the rest of the interface.

@Mibou
Copy link
Contributor Author

Mibou commented Apr 22, 2013

Ok. I will do the modifications and pull them back to you. The reason I kept filename variable was not to break retrocompatibility and I didn't want to pass my file like object through the filename variable.

Do you think it's ok if I change the filename variable to a font variable ?

@wiredfool
Copy link
Member

Looking at this a bit more, I'm not sure that file_like.buf is generally valid. It's certainly not in the declared interface for StringIO or file, where one can rely on read/write/close, and getvalue for stringio.

I think it's probably safe to rename that parameter, it might be safer to do something like:

 def __init__(self, font=None, size=10, index=0, encoding="", file=None):
       if file:
             # try raising a deprecation warning here, 
             font = file

       if isStringType(font):
             self.font = core.getfont(...)
       else: # file_like
             self.bytes = font.read()
             self.font = core.getfont( with bytes and length...)

Keeping the bytes as an instance variable should satisfy the read only buffer that's required as long as the font's in use.

At some point, I intend to grab as many packages as I can from debian and wherever else so that I can grep through them to see how PIL is being used to check compatibility. This would be something I'd like to test, greping for FreeTypeFont | grep file

@Mibou
Copy link
Contributor Author

Mibou commented Apr 25, 2013

Ok thank you. I did the changes and added the unitests. So I think it should be ok now !
Thank you for your feedback.

@wiredfool
Copy link
Member

Thanks for the update. I'll review this soon. I want to look closely and carefully.

@wiredfool
Copy link
Member

I've added a few changes in #209.

  • I've made the read buffer an instance variable for FreetypeFont, so that it has the same lifetime as the font object
  • I've renamed file_like to font_bytes in the c code, so that it sounds more like a buffer (it's just bytes at that point)
  • I've added tests.

Thanks for your contribution.

@wiredfool wiredfool closed this Apr 26, 2013
radarhere pushed a commit to radarhere/Pillow that referenced this pull request Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants