Integrate CoreFoundation with the asyncio event loop. #78
Conversation
|
Right, Travis is still broken on the main branch. There's a fix for the Travis issue in one commit that's part of #76, but I haven't had time to finish that yet... Would it be possible to implement this using Foundation APIs rather than Core Foundation? IMHO it would be better if we eventually migrated away from CF completely, see #74. I'm not very familiar with async stuff, so I can't really comment on much else, sorry. |
|
Can be done with Foundation APIs... I'm not sure. I'm mostly stumbling around trying to work out how Python's asyncio integration works, and what minimal examples exist in this space all seem to use the CoreFoundation libraries. At a guess, this is because integrating with asyncio requires fairly low level API access, and some of the object lifecycle handling around the Foundation classes can get in the way. For example, rewriting what is currently on this branch to use However, it may also be because integrating with C libraries is easier than integrating with Objective C (unless, of course, you've got Rubicon backing you up!) At the very least, it should be possible to restrict CoreFoundation usage to just the async module within Rubicon. For at least this first pass, I'd like to stick to CoreFoundation; once I've got that working, we can look at dropping CoreFoundation in revised versions. |
|
Sounds reasonable. Now that I think about it, I remember reading that some types aren't toll-free-bridged even though they have similar names. Just to be clear - are you still working on the PR, or is it finished? Because the commit says "First pieces" it sounds like there are other parts that you want to add later. (There are also no tests and docs yet.) |
|
Sorry - I forgot to apply a label. Yes, this is a work in progress. I've only got timers working so far, not the rest of communications, and as you've noted, there aren't any docs or tests yet. I'm working on those at the moment. |
|
No worries :) Just wanted to check so that I don't merge it too early. |
This is required for lifecycle management of NSApplication objects.
There was a problem hiding this comment.
As I said before, I don't know much about asyncio or Core Foundation's run loop and async APIs, so I can't judge this very well. In general everything looks good, and I assume it works well for you locally (Travis seems to be broken in a new way now...). I made a couple of comments below, but those are really minor things - other than that I would be happy to merge this.
rubicon/objc/async.py
Outdated
| CFAllocatorRef = objc_id | ||
| CFData = objc_id | ||
| CFIndex = c_long | ||
| CFOptionFlags = c_ulong |
There was a problem hiding this comment.
Some of these types are already defined in core_foundation - I think it would be better to import them from there instead of redefining them. CFOptionFlags isn't defined in core_foundation yet, but I think it would make sense to also move it there.
There was a problem hiding this comment.
Good point. I'll move and/or reuse the more generic ones.
rubicon/objc/async.py
Outdated
| ] | ||
|
|
||
|
|
||
| kCFAllocatorDefault = objc_const(libcf, 'kCFAllocatorDefault') |
There was a problem hiding this comment.
kCFAllocatorDefault is documented as being NULL, so it might be easier to simply pass None as the allocator to the CF creation functions. Of course that also makes the code a bit less readable. If we keep the constant, I think it should be moved to core_foundation as well (see above).
There was a problem hiding this comment.
I didn't see this one in the Apple docs. Agreed that using None directly makes more sense.
rubicon/objc/async.py
Outdated
|
|
||
| def cancel(self): | ||
| """ | ||
| """ |
There was a problem hiding this comment.
This should probably be removed (or filled).
There was a problem hiding this comment.
Unfortunately, this comment has been cancelled. :-)
|
Late question: what is the syntax that you're using in the docstrings? I've seen it used in other places already, but I don't know what tool it's from. Would it make sense to use Sphinx syntax in docstrings, so we could auto-generate reference docs from docstrings later? Sphinx seems to support this: https://pythonhosted.org/an_example_pypi_project/sphinx.html#auto-directives |
|
@dgelessus I'm not entirely sure what you're referring to with the doctoring format. I've been pretty much doing the "first line is a summary, subsequent lines are details" format, without any specific directives. In many cases, the comment has been lifted directly from the CPython implementation (since that definition is still accurate). I know Toga has been going a bit more detailed with argument definitions, specifically so it can be rendered as autodoc - but I haven't been as focussed on this for Rubicon. Do you think it's worth adding? |
|
Ok - this is pretty much ready now. It's tested (as much as possible), it has docs, and it has a placeholder for iOS support once I work out how that will work. Pending your final review, @dgelessus, I think we're ready to go. |
|
Sorry, I should have been more specific - I was referring to the docstring of I think autodoc would be useful for filling out the "reference" section of the docs - right now we only have the tutorials and how-tos. While the how-tos are great when using a feature for the first time, I think we should also have a reference generated from the docstrings where all constants, functions and classes are documented by module. That way when you (for example) see |
|
Ah! Yeah - that docstring is Twisted's docs format - and I agree that Sphinx's is better. I'll clean that one up. |
No description provided.