Increase performance of typer autocompletion by breaking out subcommand functions into their own module #77
Conversation
|
@MathiasSven Hmm when I test on Windows in Git-Bash (fish shell) or MacOS using Fish shell or Zsh but I don't see any autocompletion happening. Is this something that needs to be configured in Typer application? I tried the following in Windows Git-Bash and MacOS terminal using Fish Shell / Zsh:
Jack@...> git-sim --show-completion fish
complete --command git-sim --no-files --arguments "(env _GIT_SIM_COMPLETE=complete_fish _TYPER_COMPLETE_FISH_ACTION=get-args _TYPER_COMPLETE_ARGS=(commandline -cp) git-sim)" --condition "env _GIT_SIM_COMPLETE=complete_fish _TYPER_COMPLETE_FISH_ACTION=is-args _TYPER_COMPLETE_ARGS=(commandline -cp) git-sim" |
|
You need to source it in order to take effect, or use the To try it out on
|
|
@MathiasSven Gotcha that worked and I was able to test in Windows powershell/git-bash fish and MacOs Fish. FYI I don't see the manim entry in MacOS or Windows... Would another option be to conditionally import manim only when the program is executed with a subcommand like "git-sim log", and ignore the manim import if Typer is just doing its completion thing? Something like: if (git-subcommand option is present):
import manim as mIf possible, I prefer to keep the Typer command functions in the corresponding subcommand modules for ease of code organization/editing/creating new subcommands. Note: It looks like in Windows gitbash/Fish it's adding a Also, looks like your PR is based on the Can you apply your changes to the latest version of the |
That could work, taking git_sim.add as an example, the way typer registers the command is by running it through # ...
import manim as m
# ...
from git_sim.animations import handle_animations
from git_sim.git_sim_base_command import GitSimBaseCommand
from git_sim.settings import settings
class Add(GitSimBaseCommand):
...
def add(
files: List[str] = typer.Argument(
default=None,
help="The names of one or more files to add to Git's staging area",
)
):
settings.hide_first_tag = True
scene = Add(files=files)
handle_animations(scene=scene)
Will result in slow down as all of them load With your suggestion, which also seems to be the way it was worked around here, all modules would have to be mostly indented by one block inside an if statement, it should work about the same, so it would be a matter of preference... # _utils.py
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", "")# add.py
# ...
from git_sim._utils import should_define
if should_define('add'):
import manim as m
# ...
from git_sim.animations import handle_animations
from git_sim.git_sim_base_command import GitSimBaseCommand
from git_sim.settings import settings
class Add(GitSimBaseCommand):
...
def add(
files: List[str] = typer.Argument(
default=None,
help="The names of one or more files to add to Git's staging area",
)
):
settings.hide_first_tag = True
scene = Add(files=files)
handle_animations(scene=scene)Taking some inspiration from the mentioned issue |
|
@MathiasSven Thanks a lot for this research and all the details, I don't think it's an ideal solution unfortunately... Going back to your original PR, I just notice that the new def add(
files: List[str] = typer.Argument(
default=None,
help="The names of one or more files to add to Git's staging area",
)
):
from git_sim.add import Add
settings.hide_first_tag = True
scene = Add(files=files)
handle_animations(scene=scene)This actually includes an import back to Add, (as other other functions do too), so wouldn't this also require Manim to be imported recursively since Add inherits from GitSimBaseCommand? |
Signed-off-by: Mathias Sven <mathiassven2@hotmail.com>
|
Yes however that only runs once |
Signed-off-by: Mathias Sven <mathiassven2@hotmail.com>
|
I also just pushed a small commit to also delay the
|
initialcommit-io
changed the title
Fixes #76
The main problem seemed to be loading
manim. By separating the commands from the modules wheremanimneeds to be loaded, it is possible to delay loading it until a command is actually executed, since the auto-completion only relied on those command functions being registered bytyper.While the auto-completion is still a bit slow, it is a significant improvement (3x), and as a bonus because
manimisn't loaded theManim Community v0.16.0.post0log doesn't clutter the output ofgit-sim --show-completion SHELL.Previous:
slow.mp4
With PR:
faster.mp4