Skip to content

Add progress callback#74

Open
869570967 wants to merge 10 commits into
henon:mainfrom
869570967:main
Open

Add progress callback#74
869570967 wants to merge 10 commits into
henon:mainfrom
869570967:main

Conversation

@869570967

@869570967 869570967 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
  1. Expose the progress callback of Downloader.Download step by step to external interfaces.
  2. Add an Action progress parameter to RunCommand, and also add an Action progress parameter to PipInstallModule, which calls it.

Purpose: When calling functions like await Installer.TryInstallPip(); and await Installer.PipInstallModule("torch", progress: progress); sometimes, due to network issues or very large packages, the program has to wait a long time without any progress feedback. Simply showing a loading spinner isn’t ideal, so here we add a progress callback parameter to make it easier to display the current progress in real time.

PS: The code was implemented using AI, and self-testing shows no issues so far. Here's my test code—you can also try it out to see if it works. If there are no problems, it can be merged and published to NuGet (pre-release is fine too). I'm a bit pressed to use it. Thanks a lot!

using Python.Included;

namespace PythonDemo
{
    internal class Program
    {
        static async Task Main(string[] args)
        {
            Installer.LogMessage += Installer_LogMessage;
            //Console.WriteLine("Hello, World!");
            await Installer.SetupPython();
            await Installer.TryInstallPip();
            Action<float> progress = p =>
            {
                Console.WriteLine(p);
            };
            await Installer.PipInstallModule("torch", progress: progress);
        }

        private static void Installer_LogMessage(string obj)
        {
            Console.WriteLine(obj);
        }
    }
}

@869570967

Copy link
Copy Markdown
Contributor Author

Here are my test results:
image
image

@henon

henon commented Jun 24, 2026

Copy link
Copy Markdown
Owner

If you look at the changes you'll see that the AI removed English comments in some places and added Chinese ones in others. This is of course not acceptable for an international Open Source project.

Please instruct your agent never to remove any comments and always use English for its own comments. Also, it should add only comments which are necessary for understanding the code. Also instruct it to touch nothing else except what needs to be changed for the purpose of the new feature.

@henon henon self-requested a review June 24, 2026 10:42

@henon henon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighted only two examples, there are many more places where changes need to be undone.

Comment thread Python.Deployment/Installer.cs Outdated
Comment thread Python.Deployment/Installer.cs
… restore the parts of comments that were previously removed by AI.

@henon henon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some original comments are still missing and there is a question about the new parameter filename. The rest looks good.

Comment thread Python.Deployment/Installer.cs Outdated
Comment thread Python.Deployment/Installer.cs
Comment thread Python.Deployment/Installer.cs
@869570967

Copy link
Copy Markdown
Contributor Author

Line 305 in Installer.cs

@henon

henon commented Jun 25, 2026

Copy link
Copy Markdown
Owner

OK, so I made changes to keep RunCommand clean. It was intended to run shell commands only. By adding python command parsing to it the AI poisoned that method's clean and simple purpose. Instead I split off a separate RunPipCommand which is specialized for running PIP and parsing its progress. Please test if this still works for you then we can merge this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants