Lazy load code modules #269
Conversation
leofang
added
enhancement
ksimpson-work
force-pushed
the
lazy_load_code_modules
branch
from
cba8d04 to
9fba2b7
Compare
|
/ok to test |
|
/ok to test |
| @@ -7,20 +7,12 @@ | |||
| # is strictly prohibited. | |||
|
|
|||
| import pytest | |||
| from conftest import can_load_generated_ptx | |||
There was a problem hiding this comment.
My understanding is that we don't need explicit imports for things from conftest?
There was a problem hiding this comment.
I believe this only aplies to fixtures. helper functions still need to be imported. This is what my research and tests have shown.
There was a problem hiding this comment.
I could add a new file called utils, or helpers and import that instead. There is mixed opinions on best practice online
There was a problem hiding this comment.
This is what my research and tests have shown.
I was only relying on chatgpt before. It told me the imports are not needed. If that's not true: keeping it simple seems best to me, unless you anticipate that we're accumulating many helper functions. I.e. just keep what you have?
There was a problem hiding this comment.
yeah, it told me that too, then changed its mind. Keeping it simple was my idea as well, with only 1 helper function, it seems premature to split it into a new file, but perhaps down the line it will make sense
There was a problem hiding this comment.
yeah we can move it to tests/utils next time we touch this file
Co-authored-by: Leo Fang <[email protected]>
|
/ok to test |
|
FYI, the PR title can be renamed |
| if isinstance(module, str): | ||
| # TODO: this option is only taken by the new library APIs, but we have | ||
| # a bug that we can't easily support it just yet (NVIDIA/cuda-python#73). | ||
| if jit_options is not None: | ||
| raise ValueError | ||
| module = module.encode() |
There was a problem hiding this comment.
don't thank me, thank the good folks at ruff
|
/ok to test |
|
/ok to test |
load modules in CodeObject lazily
close #268