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

Support os.PathLike for paths and filenames. #150

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Conversation

Hoikas
Copy link
Member

@Hoikas Hoikas commented Sep 28, 2019

This changeset adds support for the os.PathLike protocol added in Python 3.6 for any properties or function arguments that expect anything resembling a path or a filename. This allows us to pass pathlib.Path objects seemlessly to libHSPlasma in Python 3.6+. CPython seems to use PyUnicode_FSDecoder instead of manually calling the __fspath__ method, so that is the approach I took. Unfortunately, In lower versioned Python 3.5 and lower, it is still required to convert path objects to their string representations before passing them to libHSPlasma.

This closes #148

@Hoikas Hoikas force-pushed the pypathlike branch 2 times, most recently from 0570108 to 713252f Compare September 28, 2019 01:59
Python/PyPlasma.cpp Outdated Show resolved Hide resolved
@Hoikas
Copy link
Member Author

Hoikas commented Oct 2, 2019

Ok, I've updated this to use a converter to ST::string. In doing so, I found and fixed an amusing bug in PyAnyString_Check. Also, included pySDLMgr.readDescriptors. Further, I updated a few of the type error messages. When there were a lot of arguments, I was uncertain how to clearly indicate what was going on, so thoughts there would be appreciated 😄

@@ -44,6 +44,25 @@ ST::string PyAnyString_AsSTString(PyObject* str)
}
}

int PyAnyString_PathDecoder(PyObject* obj, ST::string* str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the second argument will need to be void* to comply with Python's API...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It compiles and works on my testing in Windows. That's probably because Python's doing crazy va arg magic where type safety becomes a figment of the imagination. I can change it if you prefer, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and changed the second argument to void*.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think newer versions of GCC (GCC 8?) will complain about it (our CI is only building with GCC 5, so it doesn't trigger). So thanks for updating it.

Python/PyPlasma.cpp Outdated Show resolved Hide resolved
Python/PyPlasma.h Show resolved Hide resolved
@zrax zrax merged commit 9c41477 into H-uru:master Oct 2, 2019
@Hoikas Hoikas deleted the pypathlike branch June 15, 2020 01:40
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.

PyHSPlasma should accept os.PathLike
2 participants