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

--imports option for outputting # from xxx.yyy import function_name #26

Closed
simonw opened this issue Jun 21, 2023 · 7 comments
Closed

--imports option for outputting # from xxx.yyy import function_name #26

simonw opened this issue Jun 21, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@simonw
Copy link
Owner

simonw commented Jun 21, 2023

This will make it even easier to pipe -s output to llm and get back working code examples.

symbex -d symbex --imports

Should output:

# File: symbex/lib.py Line: 12
# from symbex.lib import find_symbol_nodes
def find_symbol_nodes(code: str, filename: str, symbols: Iterable[str]) -> List[Tuple[(AST, Optional[str])]]

# File: symbex/lib.py Line: 36
# from symbex.lib import code_for_node
def code_for_node(code: str, node: AST, class_name: str, signatures: bool, docstrings: bool) -> Tuple[(str, int)]

Also a --no-file option to suppress output of the # File: lines.

Those can have shortcuts of -i for --imports and -n for --no-code - so if you want to apply both (and just get the imports) you can then do this:

symbex -d symbex -in
@simonw simonw added the enhancement New feature or request label Jun 21, 2023
@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

The interesting challenge here is how to figure out that import string, since it depends on the location of the function relative to the Python path.

I think the following heuristics should work:

  • If the file in question lives in any of the directories currently on sys.path, figure it out relative to that
  • Otherwise, try to figure it out relative to one of the -d directory paths (including . if symbex was run without arguments)
  • For -f path/to/file.py files which are outside of anything in sys.path just go with from .module import function_name

@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

Tried those heuristics and found a problem - when using the -d option you often want to do things like -d ../datasette/tests - but that gives incorrect import statements.

Might be that it needs a --imports-relative ... option too.

@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

That --imports-relative option could be called --sys-path and could add new folders to the simulated sys.path when calculating relative imports.

Part of the reason I'm having trouble here is that -d ../datasette is also pulling in files from datasette/.eggs. Two things that would help here:

@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

This initial implementation doesn't yet do anything fancy with sys.path but it's enough to start experimenting.

simonw added a commit that referenced this issue Jun 21, 2023
simonw added a commit that referenced this issue Jun 21, 2023
Refs #26

Signed-off-by: Simon Willison <swillison@gmail.com>
@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

This does not do the right thing with class methods, *.*:

symbex '*.*' -in
# from tests.example_symbols import __init__
    def __init__(self, a)

# from tests.example_symbols import method_types
    def method_types(self, b: int) -> bool

For class methods attempting to display a from x import y line doesn't make sense at all.

What would make sense? I guess showing the name of the class, which the # File line does already:

symbex '*.*' --imports
# File: tests/example_symbols.py Class: ClassWithMethods Line: 79
# from tests.example_symbols import __init__
    def __init__(self, a)

# File: tests/example_symbols.py Class: ClassWithMethods Line: 82
# from tests.example_symbols import method_types
    def method_types(self, b: int) -> bool

@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

So what should -in do here? Maybe it should remove the File: line but add in a new Class: XXX line, perhaps like this:

# Class tests.ClassWithMethods
    def __init__(self, a)

So the class is fully qualified.

Or maybe it should look like this:

# from tests import ClassWithMethods
    def __init__(self, a)

I think that second option is more consistent with # from tests.example_symbols import method_types.

@simonw
Copy link
Owner Author

simonw commented Jun 21, 2023

A thing that worries me about this feature is that it's genuinely making promises it can't keep.

If an LLM is fed an incorrect import path it will generate incorrect example code.

If a user is browsing function documentation using this and reads an incorrect import path they'll get confused too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant