[WIP]Wildcard imports #254
Conversation
There was a problem hiding this comment.
Wow this is great! Thanks for taking this on. I have a few specific suggestions below but from a high level, I would recommend adding one or two helper methods to the runtime package so that you don't have to do so much work in the generated code.
As I'm sure you've found, it's pretty tedious and tricky to write correct generated code. You could do a lot of the work here in a simple Go function in the runtime library and avoid a lot of this complexity. E.g. something like:
// Load members scans over all the members in module
// and populates globals with them, taking __all__ into
account.
func LoadMembers(f *Frame, globals *Dict, module *Module) *BaseException
In fact, I don't even think you need to pass the globals dict, but could just use f.Globals() directly, assuming that the current frame's globals are the ones that should be populated.
compiler/stmt.py
Outdated
|
|
||
| self.writer.write_checked_call2(members_iterator, 'πg.Iter(πF, {})', members.expr) | ||
|
|
||
| loop = self.block.push_loop() |
There was a problem hiding this comment.
The reason the loop stack exists is to support Python break and continue statements. Since you're generating a fixed block of code, you don't need to push a new loop.
That said, maybe it's easier since it gives you a start and end label to work with? My concern is that the loop stack is currently only used to manage Python loops and I'm not 100% sure there won't be some weird interaction between this code and Python loops.
| members_iterator=members_iterator.expr, | ||
| end_label=loop.end_label) | ||
|
|
||
| with self.block.resolve_name(self.writer, 'getattr') as getattr_method, \ |
There was a problem hiding this comment.
You can just call πg.GetAttr() instead of fetching the getattr Python object.
| from re import * | ||
| print 'sre_compile' in globals()"""))) | ||
|
|
||
| # 'time' does not define __all__ |
There was a problem hiding this comment.
This seems like an implementation detail that could change. Maybe we need to create a couple dummy test modules for the purposes of testing this functionality.
| """ | ||
| module_name = node.module | ||
|
|
||
| with self.block.alloc_temp() as members, \ |
There was a problem hiding this comment.
IIUC members is a temporary that will be filled up, used to populate the globals dict and then thrown away. I would just populate the globals dict as I iterate over the imported module's members and avoid the temporary.
|
It struck me that I could write the code in Go, as I got more familiar with the codebase. It was a bit too late and I was closer to finishing the Python code. And I was afraid Go would be the better solution. :) I'll start working on that on Monday, excited about how this will turn out. BTW, the Travis jobs for my commits seem to take a lot of time. Job #679 seems to have been running for over 50 mins now. Is that normal? |
|
Sounds good.
The OS X job on Travis is slow. Usually it suffices to validate that the
Linux job was successful. It's usually pretty quick.
…On Thu, Feb 16, 2017 at 11:23 AM Claudiu Coman ***@***.***> wrote:
It struck me that I could write the code in Go, as I got more familiar
with the codebase. It was a bit too late and I was closer to finishing the
Python code. And I was afraid Go would be the better solution. :) I'll
start working on that on Monday, excited about how this will turn out.
BTW, the Travis jobs for my commits seem to take a lot of time. Job #679
seems to have been running for over 50 mins now. Is that normal?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#254 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAHLe9R6fksWkmB1hIUCH4CLAYQemholks5rdKIhgaJpZM4MDVBG>
.
|
Hi @trotterdylan! I read the conversation in #192 and I thought I'd take a shot at wildcard imports. The code should be functional, I've updated the tests too.
If the idea is the right one, the next step for me would be to start trimming that code down. I can isolate some code in reusable methods and remove some of the duplication.
Before I take this PR further, can you tell me if I'm heading in the right direction?