-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Option to exclude code #67
Comments
Why not, but I'd be interested in seeing your use case for this |
I actually realised I didn't need this functionality in the end. Possibly not so useful? |
@pquentin one example would be if |
@pquentin I would find this useful for my projects... For the time being, I just post-process my code to get the same effect. A key use-case is for when the async/sync implementation is fundamentally different... particularly when I need the resulting code to pass As a small example, I have two functions that provide sync/async implementations: async def async_create_process(runargs: list[str]) -> asyncio.subprocess.Process:
return await asyncio.subprocess.create_subprocess_exec(*runargs, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
def sync_create_process(runargs: list[str]) -> subprocess.Popen[bytes]:
return subprocess.Popen(runargs, stdin=subprocess.PIPE, stderr=subprocess.PIPE, stdout=subprocess.PIPE) In my async code that gets processed by The problem is that, after rewriting I end up with this: def sync_create_process(runargs: list[str]) -> asyncio.subprocess.Process:
return asyncio.subprocess.create_subprocess_exec(*runargs, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
def sync_create_process(runargs: list[str]) -> subprocess.Popen[bytes]:
return subprocess.Popen(runargs, stdin=subprocess.PIPE, stderr=subprocess.PIPE, stdout=subprocess.PIPE) Technically, this code works because the second function overwrites the first one and all callsites for To fix this, I created a post-processor that looks for So if I add the comment like this... async def async_create_process(runargs: list[str]) -> asyncio.subprocess.Process: # unasync: remove The resulting code has just the sync implementation... def sync_create_process(runargs: list[str]) -> subprocess.Popen[bytes]:
return subprocess.Popen(runargs, stdin=subprocess.PIPE, stderr=subprocess.PIPE, stdout=subprocess.PIPE) And Implementing this might also be a solution/workaround for issues like #74 and #64 If anyone is interested in the (largely untested) code, it's available in this gist |
Thanks for the detailed explanation! Yeah mypy wasn't as common when we started unasync. I would be happy to merge a pull request to that effect. @spyoungtech Note that to reuse your code in unasync it needs to be licensed as MIT or Apache 2. One thing I'm wondering about: both use cases in that issue are about removing a function altogether, so a single marker above that function is enough and can even cover classes I think? On the other hand, having beginning/end markers instead ( |
The way it is implemented in the gist is that it removes the entire node on the comment. Similar to how you may mark things like # remove entire lines:
foo: AsyncOnlyType = AsyncOnlyThing() # unasync: remove
foo += bar + (bacon, eggs, spam) # unasync: remove
# functions
async def foo(...): # unasync: remove
print('this all')
print('will be removed, in the sync version')
# remove an entire class
class AsyncFoo: # unasync: remove
...
# or just specific methods:
class Bar:
async def foo(self, ...): #unasync remove
...
# or a line within a method
def foobar(self, ...):
asyncio.run(...) # unasync: remove Begining/end markers might be useful for modules with a ton of module level constants if they can't just group the statements in parens. In most cases, I would suspect something like this is possible instead of
and that's probably a simple implementation (though I'm not sure it's the most obvious) The code in the gist is MIT licensed, so feel free to use it. I'd also be happy to make a PR for this, but I'm not sure how to elegantly incorporate it to the current process. It also requires |
So yeah it looks like
MIT is good, but Apache 2 has to be an option too, this is how all/most Trio projects are licensed. https://github.com/python-trio/unasync/blob/master/LICENSE
If it works and is tested, it does not have to be super elegant? I have not looked at this at all.
Oh, did not know about that one, it's nice! Definitely noticed the escaped lines issue when testing unasync on the standard library. Switching to |
For whatever it's worth, my changes in #75 have been successfully been used in my project ahk for about a year and a half now. I think unlike most users, I actually commit the generated code, so the Some other interesting use cases I've employed... For example as seen here you can write distinctly different sync/async code more easily by using this to reliably know whether your method/function/whatever is running under async or not: def some_method(self) -> Any:
is_async = False
is_async = True # unasync: remove
if is_async:
... # behavior you want when run under async variant of the class
else:
... # behavior you want when run in the synchronous variant of the class
... It can be used to define things that only work after being unasynced, for example raising an error in the async API, but removing it so it runs without exception in the sync API: @mouse_position.setter
def mouse_position(self, new_position: Tuple[int, int]) -> None:
raise RuntimeError('Use of the mouse_position setter is not supported in the async API.') # unasync: remove
x, y = new_position
return self.mouse_move(x=x, y=y, speed=0, relative=False) Same idea can be used for warnings that may only apply in async code. I think #75 is stalled, but I'm available if those changes want to be revisited. |
Thanks for the feedback. I'm happy to include the feature, but don't have the reviewing cycles at the moment. Having someone to make CI pass (you?) and then someone to review that work would help. |
No worries @pquentin thanks for your response. I've addressed the merge conflicts and the PR is now green. |
Something like:
# unasync: ignore
and / or exclusion patterns..
See https://coverage.readthedocs.io/en/coverage-5.3/excluding.html
The text was updated successfully, but these errors were encountered: