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

Extended the examples of string.rst and put them in the examples directory for testing. #2361

Merged
merged 2 commits into from Jun 17, 2018

Conversation

gabrieldemarmiesse
Copy link
Contributor

I added examples of manipulation of strings in Cython (the c_func.pyx and c_func.pxd) because I think it's a good thing to show more cython examples (even if they are trivial).

If you don't want them in the docs, we can keep them in the examples directory (for the tests) and not display them in the documentation.

# A type cast to `object` or `bytes` will do the same thing:
py_string = <bytes> c_string

free(c_string)
Copy link
Contributor

Choose a reason for hiding this comment

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

This definitely needs a try-finally, because the object creation might fail with MemoryError.
And since that adds even more complexity to an originally very simple piece of example code, I'd rather leave this specific 2-line snippet untouched.

# get pointer and length from a C function
get_a_c_string(&c_string, &length)

py_bytes_string = c_string[:length]
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a try-finally.

try:
py_bytes_string = c_string[:length] # Performs a copy of the data
finally:
PyMem_Free(c_string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, no, please use plain malloc() and free() here. The intention is to show how to interact with something C-ish (e.g. an external C library), not with something that understands the CPython C-API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I though it would be a good example. I'll change it.

@scoder scoder merged commit 46b3955 into cython:master Jun 17, 2018
@gabrieldemarmiesse gabrieldemarmiesse deleted the test_string branch June 17, 2018 14:31
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