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

Faster Auto-completion #231

Open
nkitsaini opened this issue Feb 1, 2021 · 19 comments
Open

Faster Auto-completion #231

nkitsaini opened this issue Feb 1, 2021 · 19 comments
Labels
feature New feature, enhancement or request investigate

Comments

@nkitsaini
Copy link

nkitsaini commented Feb 1, 2021

Is your feature request related to a problem

Typer auto-completion becomes slower as the project grows. In my sample application, having only a few imports increases the response time above 100ms. This affects the user experience.

The solution you would like

If there could be some way of defining auto-completion functions in different file, we can only have imports necessary for auto-completion.

I provide a proposal below, but I am not too confident in it due to type-hint discontinuity. This just serves as starting point.

Additional context

""" Calculation of response time """
import time
start = time.perf_counter()
import subprocess # necessary for autocompletion
import typer 
import rich.console # not necessary
import pystray # not necessary
import pynput # not necessary
import watchgod # not necessary

print(1000 * (time.perf_counter() - start)) # ~104
""" Draft of proposed API """
## main.py
from typer import Typer, Argument
app = Typer(completion_module="./completion.py")

@app.command()
def branch_autocompletion(name: str = Argument(autocompletion_function="branch_names")): ...

## completion.py
from typer import CompletionModule
def branch_names():
    return ("feature", "bug")
completion = CompletionModule({'branch_names': branch_names})
@nkitsaini nkitsaini added the feature New feature, enhancement or request label Feb 1, 2021
@alextremblay
Copy link

with your proposed api, typer would still need to import main.py in order to get the list of commands and completion functions, which means it would still need to import all the modules and packages imported in main.py, and all of those packages' dependencies, and so on

I don't see how that would improve performance.

One possible workaround would be to refactor your cli entrypoint module (main.py) to lazy-load its dependencies only when needed

i.e. instead of:

## main.py
from typer import Typer, Argument
import rich.console # not necessary
import pystray # not necessary
import pynput # not necessary
import watchgod # not necessary

app = Typer()

def some_command_autocompletion():
    # do completion
    pass

@app.command()
def some_command(name: str = Argument(autocompletion=some_command_autocompletion)): 
    # do stuff
    pass

you could maybe do something like:

## main.py
from typer import Typer, Argument

app = Typer()

def some_command_autocompletion():
    # do completion
    pass

@app.command()
def some_command(name: str = Argument(autocompletion=some_command_autocompletion)): 
    import rich.console # not necessary
    import pystray # not necessary
    import pynput # not necessary
    import watchgod # not necessary
    # do stuff
    pass

@nkitsaini
Copy link
Author

Thanks for reply. This approach makes sense but sometimes just to type hint the code we'll have to make global import.

def press_key(key: Union[str, pynput.Keyboard.key]):
     ....

This will require me to have pynput as global import.
I should've been more clear as In the proposed API all the commands should be processed when user runs cli --install-completions and then stored in some structure in bash/fish... completion files. And as I said even I'm not satisfied with my approach due to type-hint discontinuity.

@alextremblay
Copy link

I see...

So with your proposed API, typer would parse and cache all commands and their completion functions only once, when --install-completions is called?

How would you manage that cache? how would typer know when that cache needs to be invalidated?

@nkitsaini
Copy link
Author

This is kind of what I have in mind.

# psuedo structure inside shell completion file.
commands = {
    "git": ["branch", "commit"],
    "git branch": ["function:branch_list"],
    "git commit": ["-m"],
}

We store command names and completion function names. While completion, if it's a function name then get completion values by executing module completion.py. We don't need to worry about cache in this case.

@sidekick-eimantas
Copy link

As a quick hack, I did the following in my application:

Given this layout:

skm_cli/_cli
├── __init__.py
├── _agent.py
├── _aws.py
├── _cli.py
├── _dmarc.py
├── _project.py
├── _publish.py
├── _slack.py
├── _tf.py
└── _utils.py

_cli.py looks as follows:

import typer

from skm_cli._cli import _agent
from skm_cli._cli import _aws
from skm_cli._cli import _dmarc
from skm_cli._cli import _middleware
from skm_cli._cli import _project
from skm_cli._cli import _publish
from skm_cli._cli import _slack
from skm_cli._cli import _tf

app = typer.Typer(
    no_args_is_help=True,
    pretty_exceptions_enable=False,
)
app.add_typer(_agent.app, name="agent")
app.add_typer(_aws.app, name="aws")
app.add_typer(_dmarc.app, name="dmarc")
app.add_typer(_publish.app, name="publish")
app.add_typer(_project.app, name="project")
app.add_typer(_slack.app, name="slack")
app.add_typer(_tf.app, name="tf")

_utils.py looks like this:

import sys
import os


def should_define(command: str) -> bool:
    return _cli_is_invoking_command(command=command) or _autocomplete_is_resolving_command(command=command)


def _cli_is_invoking_command(command: str) -> bool:
    return command in sys.argv


def _autocomplete_is_resolving_command(command: str) -> bool:
    return command in os.environ.get("_TYPER_COMPLETE_ARGS", "")

_agent.py, _aws.py and all other Typer subcommand files look like this:

import typer

from skm_cli._cli import _utils

app = typer.Typer(no_args_is_help=True)

if _utils.should_define(command="dmarc"):
    ...  # all commands are defined below

The end result is - all cli commands are lazily defined. Given some heavy imports, my app autocompletes would clock it at around 0.5s. With this hack, they're down to 0.1s

@NikosAlexandris
Copy link

NikosAlexandris commented Dec 12, 2023

skm_cli/_cli
├── __init__.py
├── _agent.py
├── _aws.py
├── _cli.py
├── _dmarc.py
├── _project.py
├── _publish.py
├── _slack.py
├── _tf.py
└── _utils.py

Pardon me for my ignorance @sidekick-eimantas : what is your benefit in prefixing all modules with an underscore ?

@sidekick-eimantas
Copy link

skm_cli/_cli
├── __init__.py
├── _agent.py
├── _aws.py
├── _cli.py
├── _dmarc.py
├── _project.py
├── _publish.py
├── _slack.py
├── _tf.py
└── _utils.py

Pardon me for my ignorance @sidekick-eimantas : what is your benefit in prefixing all modules with an underscore ?

Hey Nikos

This is a convention we use, which is an extension of https://docs.python.org/3/tutorial/classes.html#private-variables, applied to modules. We define the interface for the cli package in the __init__.py file, which looks like:

# CLI Package
# The only interface here is `app`
# Call the `app` method to hand off control to the CLI.

from skm_cli.cli._cli import app as app

The underscore signals to the consumer of the package not to import the modules directly, but instead to look at the __init__.py file to understand the interface of the package.

It's tedious and I don't recommend it for small codebases

@NikosAlexandris
Copy link

It's tedious and I don't recommend it for small codebases

Thank you @sidekick-eimantas . What are then (other?) benefits other than safeguarding the 'consumer' from no meaningful import(s, is my guess) ? I do have a somewhat complex use-case here, so maybe I am a candidate to replicate your approach ?

@sidekick-eimantas
Copy link

It's tedious and I don't recommend it for small codebases

Thank you @sidekick-eimantas . What are then (other?) benefits other than safeguarding the 'consumer' from no meaningful import(s, is my guess) ? I do have a somewhat complex use-case here, so maybe I am a candidate to replicate your approach ?

It's primarily just a means of documentation. Akin to exports in other languages like Erlang. Rather than having to look through the code of large modules or read external documentation to understand what are the interfaces of a module/package, the consumer only has to look at __init__.py. Internally, it gives you a bit more freedom to restructure and remodel your system without impacting the interfaces.

Hope that helps.

@NikosAlexandris
Copy link

That gives an idea. Thank you for your invaluable time to respond.

@alextremblay
Copy link

Thank you @sidekick-eimantas , that's a very nice architecture

somewhat similar to my approach, in that modules and packages are only imported when needed.

It's a very nice workaround, but i wish there was a more universal solution...

@tiangolo, what do you think about typer looking for and introspecting *..pyi files?

My thinking here is that when performing autocomplete, typer doesn't rerally need to import / process function bodies and all their respective dependencies, it only needs the function signatures in order to perform autocomplete.

it would be awesome if typer could (during autocomplete) pull these signatures from .pyi files instead of importing and introspecting .py modules themselves, and in so doing would massively cut down the time it takes to auto-complete tasks

if typer is asked to autocomplete a function argument that has a defined completetion function in its signature, typer would still need to import that specific module and execute that function, but other than that, we'd see a huge increase in completion performance / responsiveness i think.

Another benefit of leveraging .pyi files for this is that we wouldn't have to reinvent the wheel. There are already several existing dev tools which allow us to generate .pyi files for the code in our typer-powered codebases :)

@sidekick-eimantas
Copy link

I've had another look at this thread, a year wiser since last I replied with a suggestion, and am wondering if as a quick potential fix for this, typer.Typer.add_typer could take a module path string as the first parameter and import the command only if required at runtime? Similar to how Uvicorn does it (for different reasons)

import typer

app = typer.Typer(
    no_args_is_help=True,
    pretty_exceptions_enable=False,
)
app.add_typer("skm_cli._cli._agent:app", name="agent")
app.add_typer("skm_cli._cli._aws.app:app", name="aws")

@rupurt
Copy link

rupurt commented Jan 3, 2024

Click supports the concept of a LazyGroup. It would be great if we could leverage that in Typer https://click.palletsprojects.com/en/8.1.x/complex/#lazily-loading-subcommands. Also running into similar issues pretty quickly as the CLI interface grows.

I like your suggestion @sidekick-eimantas. As you mention it's a familiar pattern from uvicorn.

hlmore added a commit to INM-6/beaverdam that referenced this issue Apr 10, 2024
Autocompleting the commands was insanely slow.  Putting imports
inside the commands helped.  I got the idea from this issue:
fastapi/typer#231
@arogozhnikov
Copy link

Since 2021 speed issue now is not just due to custom code / imports, but typer itself became quite slow, for instance just running import like this

time python -c 'import typer'

takes ~200ms (!) on high-end ubuntu 22.04 server with python3.11 with typer==0.12, while typer 0.9 takes ~85ms (and 40 ms is python startup time).

It is ~20 percent faster on mac, but story is the same.

This affects autosuggestion experience of course.

@KyleTheScientist
Copy link

KyleTheScientist commented May 1, 2024

I'm also seeing very long load times for the typer module on Windows, anywhere between .2 and .5 seconds, making the autocomplete process very sluggish.

from time import time
now = time()
import typer
print(f"Imported in {time()-now}s")
Imported in 0.4669983386993408s
Imported in 0.4967198371887207s
Imported in 0.20246219635009766s
Imported in 0.2098560333251953s
Imported in 0.21218657493591309s

@amarder
Copy link

amarder commented May 24, 2024

I'm seeing similar speed issues on windows:

$ time python -c 'print("hello")'
hello

real    0m0.092s
user    0m0.000s
sys     0m0.000s

$ time python -c 'import typer'

real    0m0.417s
user    0m0.000s
sys     0m0.000s

@iamthebot
Copy link

The biggest culprit of slow typer import time seems to be rich.
Screenshot 2024-08-26 at 7 08 32 PM

(this is on OSX but similar graph on Linux)

Also see this discussion where @JPHutchins has addressed some of this upstream in Rich. Problem is their fix is merged but rich hasn't cut a release in 6+ months...

@NikosAlexandris
Copy link

@iamthebot What is the tool behind the time-profiling ?

@alextremblay
Copy link

tuna

It’s pretty awesome

pip install tuna
python -X importtime <my script/module> 2>importtime.log
tuna importtime.log

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

No branches or pull requests

10 participants