Use Click to parse command-line arguments in lemke.py by nataliemes · Pull Request #10 · gambitproject/lemke

nataliemes · 2026-05-23T18:23:59Z

This PR replaces manual command-line argument parsing with Click (in lemke.py).

Changes in detail

Instead of using global variables, main() accepts arguments (populated by Click), which are then passed onto runlemke().
If lcpfilename argument was not a valid file path, the program used to raise an unhandled exception. Now, this argument is checked by Click and if the file does not exist, is not readable, or a directory is passed instead of a file, a clean CLI error is shown automatically.
Options follow POSIX convention: single-character flags use a single dash (-v), while multi-character flags use double dashes (--verbose).
Added -h as a help option name.
Input filename is now a required argument.
runlemke() accepts an output_stream argument instead of silent - stdout is passed for console output, a file handle for file output, or None (default) to suppress all output.
Added tests for lemke.py.

Copilot

Pull request overview

This PR migrates the lemke CLI in src/lemke/lemke.py from manual sys.argv parsing to Click, aiming to improve help/usage output and validate the input file path up front.

Changes:

Replaced manual argument parsing with a Click-based main() command (options + lcpfilename argument).
Updated tableau.runlemke() to accept lcpfilename and derive the .out filename from it.
Added click as a runtime dependency in pyproject.toml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`src/lemke/lemke.py`	Introduces Click CLI decorators/options/argument parsing and threads `lcpfilename` into `runlemke()` for `.out` generation.
`pyproject.toml`	Adds Click as a project dependency to support the new CLI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

stengel · 2026-05-24T08:24:25Z

+    "lcpfilename",
+    default="lcp",
+    required=False,
+    type=click.Path(exists=True, readable=True),


@nataliemes good suggestion

+        outfile = lcpfilename + ".out"
+
        if silent:
            filehandle = open(outfile, "w")  # noqa: SIM115
        n = self.n


    printout(f"verbose={verbose} lcpfilename={lcpfilename} silent={silent} z0={z0}")
    # printout (f"{verbose}= {lcpfilename}= {silent}= {z0}=")
    m = lcp(lcpfilename)
    printout(m)
    printout("==================================")
    tabl = tableau(m)
-    tabl.runlemke(verbose=verbose, z0=z0, silent=silent)
+    tabl.runlemke(verbose=verbose, z0=z0, silent=silent, lcpfilename=lcpfilename)


stengel · 2026-05-24T08:19:51Z

-def main():
-    processArguments()
+@click.command(
+    context_settings={"help_option_names": ["-?", "--help"]},


add "-h" as another help option name

stengel · 2026-05-24T09:30:53Z

+        outfile = lcpfilename + ".out"
+
        if silent:
            filehandle = open(outfile, "w")  # noqa: SIM115


I think when silent==True there should not be any output at all (except for error messages),
including no output to file. This will also avoid the open-file handle leak risk noted by copilot.
The standard output to file is recording the pivots, if verbose==True also the intermediate tableaus.
We have to look at lemke.py as well as check for further parameters, later.

So, when silent is True, there should be no output at all (neither to a file, nor to the console). But when silent is False, should we print to both, or only to a file?

stengel

looks very good. See my suggestions, e.g. from copilot - reject directories,
and no output if silent==True.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tturocy · 2026-05-26T15:39:09Z

Some guerrilla comments on instrumenting the command-line tool, based on how command-line tools typically work:

Don't allow a default for the input filename - it should be a required argument.
You might consider allowing "-" as an input filename, in which case the data are read from standard input, useful for piping
I would suggest NEVER suppress output to stdout. If someone really wants to suppress that then you pipe to /dev/null.
If you allow a logfile to be generated, the logfile might be more detailed than the stdout output.
Think about all of the things you might include in/include out of a detailed logfile, and have command-line switches (or a list of arguments) for these.

Specifically for Python:

Try very hard to use context managers for opening files. Certainly do NOT use global variables, never never ever!

Looking ahead, a logical refactoring step is going to be to have callback functions to the main lemke routine, which will be the mechanism where you'll actually write to output if desired - you won't have any printing going on in lemke proper. But that is a few steps down the road - but the above will move in the direction of making that easy.

stengel · 2026-05-27T09:26:05Z

Don't allow a default for the input filename - it should be a required argument.

Fine, use perhaps [filename].lcp and [filename].log - see below.

You might consider allowing "-" as an input filename

No

I would suggest NEVER suppress output to stdout. If someone really wants to suppress that then you pipe to /dev/null.

when lemke is used as a library, we don't want to write to stdout. We write to stderr if the log file cannot be opened (presumably due to missing permissions).
Any progress reports during the computation should also be written, if requested, to stderr.
Question is what kind of stdout result we use when using lemke standalone from the CLI.
I think we have to iterate here, rather than aiming for the perfect solution now.

If you allow a logfile to be generated, the logfile might be more detailed than the stdout output.

maybe we should indeed call it [filename].log and the main file [filename].lcp

Think about all of the things you might include in/include out of a detailed logfile

we will do exactly that when we look at the algorithm. The current CLI arguments are just a start.

Try very hard to use context managers for opening files

That's using "with" as in

with open("file.txt", "r") as f:
      content = f.read()

and the question is how to use that together with click which is in effect the main program calling lemke when using the CLI. There must be a standard solution for this.

tturocy · 2026-05-27T09:35:03Z

When this is used as a library we will have gotten to the point where all of the writes are done by callbacks, as I noted in my previous comment, so lemke will not know anything about whether a file is being written, a data structure is being built, user interaction is happening, whatever.

Being able to read from standard input can be very useful so you don't have to create temporary files.

Filename suffixes shouldn't be assumed, they should be specified explicitly on the command line - leave it to the user how they want to do it.

Using a context manager is straightforward, wrap the call to runlemke in it:

with open(output_filename, "w") as f:
   runlemke(<arguments, which would include f as the output stream>)

Eventually we will just pass callback(s) to runlemke and hide the stream entirely, and then there are various ways to encapsulating the file opening to ensure RAII....

nataliemes · 2026-05-28T19:35:19Z

Here are the recent changes I made (although not sure if this is what you had in mind):

Added an upper bound for Click dependency
Added -h as a help option name
Made lcpfilename argument reject directories and removed its default value
For output:
Previously everything was printed to console, and if we set silent to True, then the output was sent to a file instead. As I understood, silent option worked like this, so that when runlemke(silent=True) was called in code, it wouldn't output everything to console and run quietly while writing to a file (like it was done in bimatrix.py).
In the current version, runlemke() accepts an output_stream argument instead of silent. You pass stdout for console output, a file handle for file output, or None (default) to suppress all output – this last option is used in tests and also in bimatrix.py (but if we want to output to a file like before, we can do that as well).
The CLI entry point passes stdout to runlemke(). I didn't add a Click option that would work like silent and send all output to a file instead, since I think the user could do that simply by: lemke lcp > lcp.out.

tturocy · 2026-05-29T11:32:24Z

 class tableau:
    # filling the tableau from the LCP instance Mqd
    def __init__(self, Mqd):
+        self.output_stream = None


Storing the stream as state in the tableau is not great. However, this is more of a symptom of a deeper problem than a problem in itself - the tableau class mixes confuddles up the tableau object with the lemke algorithm, which are logically distinct. Further refactorings will solve this, so I think this is OK for now because the current commit is about getting click to be responsible for the command-line parsing.

tturocy · 2026-05-29T11:33:55Z

 version = "0.0.1"
 dependencies = [
    "numpy>=2.2,<2.3",
-    "matplotlib>=3.10,<3.11"


Upper bounds on packages should only be used in the event of known incompatibilities, especially when working with mature packages (like numpy, matplotlib, click all are). Upper bounds can cause version conflicts when installing alongside other packages in the same environment.

# Conflicts: # src/lemke/lemke.py

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

nataliemes · 2026-07-04T17:49:25Z

+        self.output_stream = output_stream
+


This should be fixed after introducing callbacks: runlemke() will have a callback argument, which by default is None. If the user wants to print output, the output stream as well as the other output-related flags will be configured inside the callback object and that callback will then be passed to runlemke(). And in the callback, we can make the stream a required argument, while other flags stay optional.

+@click.option(
+    "-v",
+    "--verbose",
+    is_flag=True,
+    help="Printout intermediate tableaus",
+)
+@click.option(
+    "--z0",
+    is_flag=True,
+    help="Show value of z0 at each step",
+)
+@click.argument(
+    "lcpfilename",
+    type=click.Path(exists=True, readable=True, file_okay=True, dir_okay=False),
+)


+    """
+    Tool for solving linear complementarity problems using Lemke's algorithm.
+
+    [LCPFILENAME] is the path to the input file
+    """


+# ---   CLI   -------------------------------------------------------
+@pytest.mark.parametrize("extra_args", [
+    [],
+    ["--verbose", "--z0"],
+])
+def test_cli_runs_without_error(tmp_path, extra_args):
+    file_path = tmp_path / "test.lcp"
+    file_path.write_text("n= 2\nM= 1 0 0 1\nq= 1 1\nd= 1 1\n")
+
+    runner = CliRunner()
+    result = runner.invoke(main, [str(file_path)] + extra_args)
+
+    assert result.exit_code == 0


nataliemes · 2026-07-04T17:44:54Z

 dependencies = [
-    "numpy>=2.2,<2.3",
-    "matplotlib>=3.10,<3.11"
+    "numpy>=2.2",
+    "matplotlib>=3.10",
+    "click>=8.1",
 ]


Ignoring this, since upper bounds were removed based on the earlier review.

rahulsavani

Looks close to being ready, but if you could just respond to the copilot comments, either by fixing or ignoring and commenting why that would be great. Them please re-request a review from me.

rahulsavani

Looks good to me, leaving for Bernhard.

Use Click to parse command-line args in lemke.py

fadb0a4

nataliemes requested review from rahulsavani and stengel May 23, 2026 18:54

rahulsavani requested a review from Copilot May 23, 2026 19:05

Copilot started reviewing on behalf of rahulsavani May 23, 2026 19:05 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

stengel reviewed May 24, 2026

View reviewed changes

stengel requested changes May 24, 2026

View reviewed changes

nataliemes and others added 3 commits May 24, 2026 14:42

Add upper bound to Click dependency

fd77634

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Ensure "lcpfilename" argument accepts only files

76547ee

Add "-h" to help option names

04a3e94

nataliemes added 2 commits May 27, 2026 22:52

Remove default value for "lcpfilename" argument

ab89698

Pass output stream explicitly to runlemke()

f921095

nataliemes requested a review from stengel May 28, 2026 19:42

tturocy self-requested a review May 29, 2026 11:29

tturocy requested changes May 29, 2026

View reviewed changes

Remove upper bounds for dependencies

9698add

nataliemes changed the title ~~Use Click to parse command-line arguments~~ Use Click to parse command-line arguments in lemke.py Jun 4, 2026

nataliemes added 2 commits June 20, 2026 17:58

Merge remote-tracking branch 'origin/main' into refactor/click-cli

2be8eed

# Conflicts: # src/lemke/lemke.py

Add tests

bf78ff4

rahulsavani requested a review from Copilot July 3, 2026 10:59

Copilot started reviewing on behalf of rahulsavani July 3, 2026 10:59 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

rahulsavani reviewed Jul 3, 2026

View reviewed changes

Add CLI tests for invalid path inputs

32dc4d8

Show LCPFILENAME as a required argument in docstring

ab263d1

nataliemes requested a review from rahulsavani July 4, 2026 17:51

rahulsavani approved these changes Jul 6, 2026

View reviewed changes

stengel merged commit de955c6 into gambitproject:main Jul 6, 2026
8 checks passed

nataliemes deleted the refactor/click-cli branch July 23, 2026 05:45

Uh oh!

Conversation

nataliemes commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes in detail

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stengel May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stengel left a comment

Choose a reason for hiding this comment

Uh oh!

tturocy commented May 26, 2026

Uh oh!

stengel commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tturocy commented May 27, 2026

Uh oh!

nataliemes commented May 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rahulsavani left a comment

Choose a reason for hiding this comment

Uh oh!

rahulsavani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nataliemes commented May 23, 2026 •

edited

Loading

stengel May 24, 2026 •

edited

Loading

stengel commented May 27, 2026 •

edited

Loading