search

typemytype / drawbot

http://www.drawbot.com
Other
408 stars 63 forks source link

adds support for pathlib.Path objects for a few functions #454

Closed roberto-arista closed 2 years ago

roberto-arista commented 2 years ago

in relation to issue #449

roberto-arista commented 2 years ago

to be honest, I don't know why these older commits show up in this pull request. I forked the repo a while ago, today I realigned the existing fork with fetch upstream and then I made the edits to the code. Hopefully it is not a problem, if necessary I can start from scratch

justvanrossum commented 2 years ago

Maybe you fetched upstream after you created the PR branch?

roberto-arista commented 2 years ago

Maybe you fetched upstream after you created the PR branch?

I don't think so, but also I am not 100% sure. I am not very familiar with pull requests from forks

justvanrossum commented 2 years ago

Can you please reread my feedback and ask if it's not clear?

I wrote isinstance(path, (str, os.PathLike)) in some places, and path = os.fspath(path) unconditionally in the others.

I'm surprised the tests even pass right now, as you're not testing for str anymore.

roberto-arista commented 2 years ago

Sorry, I thought os.PathLike worked as an umbrella for both. I misread your example. I'll fix it on Monday

justvanrossum commented 2 years ago

Perfect, thanks!

roberto-arista commented 2 years ago

🎉

On 5 Dec 2021, at 11:25, Just van Rossum @.***> wrote:

 Perfect, thanks!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.