omni-us / jsonargparse

Implement minimal boilerplate CLIs derived from type hints and parse from command line, config files and environment variables
https://jsonargparse.readthedocs.io
MIT License
314 stars 42 forks source link

Read the standard input and write in the standard output #463

Closed mquillot closed 5 months ago

mquillot commented 6 months ago

🚀 Feature request

Permits to specify standard input with Path_fr

Motivation

Sometimes, I would like to be able to create a full pipeline of commands which communicate through the standard input and output. But I would also like that run each command with input file instead of standard input. (same for output)

Pitch

jsonargparse don't permits it today with Path_fr. (or I didn't find the feature?) For instance, if I put "-" (common practice for specifying standard input) with the corresponding option, it will answer that the file does not exist.

Here an example of code:

parser = ArgumentParser()

parser.add_argument(
    "--input-file",
    dest="input_file",
    type=Path_fr,
    default="-",
    help="Path of input file (standard input if - specified)",
)

The command I run: python -m my_command --input-file -

And the error I receive: Not of type Path_fr: File does not exist: /folderA/folderB/-

But, in this situation, I would like that the program does not check the existence of the file as I would like to read the standard input.

It could be great to have the same feature for the output also.

Alternatives

I tested also to use the FileType of argparse as a type, but jsonargparse does not accept it.

from argparse import FileType

parser.add_argument(
    "--input-file",
    dest="input_file",
    type=FileType("r"),
    default="-",
    help="Path of input file (standard input if - specified)",
)

Error: error: Validation failed: Parser key "input_file": expected str, bytes or os.PathLike object, not _io.TextIOWrapper

mauvilsa commented 6 months ago

@mquillot thank you for the proposal! I did have this in my TODO list, though had not gotten to it.

mquillot commented 6 months ago

I think in the following file : https://github.com/omni-us/jsonargparse/blob/main/jsonargparse/_util.py

You can just change class Path to accept "-" as a possible value skipping the check. You can also add a mode that path_type will transmit. The mode could be specified with the character "-" specifying that we accept standard input or output. For instance, you could have the following code:


        elif isinstance(path, (str, os.PathLike)):
            if "-" in mode and path == "-":
               # Do something here...
               # add a flag?
               self._skip_check = True
            else:
              path = os.fspath(path)
              cwd = os.fspath(cwd) if cwd else None
              abs_path = os.path.expanduser(path)
              if self._file_scheme.match(abs_path):
                  ...

What do you think?

mauvilsa commented 6 months ago

There is no need for a new mode. Also don't set _skip_check to true since that has other consequences. Just the if "r" in mode ... should not raise TypeError when the value is dash. The same for "w". Also get_content should support reading from stdin. And finally, there should be new unit tests to cover all of this.

mauvilsa commented 5 months ago

@mquillot are you interested in contributing this feature? Just asking to not work on this. Otherwise, I might implement this soon.

mquillot commented 5 months ago

I developed it on my side:

class PathWithSTD(Path):
    """Path class that manages standard input"""

    def __init__(self, v: str, mode: str, **k):
        if v == "-":
            self._relative = "-"
            self._absolute = "-"
            self._cwd = ""
            self._mode = mode
            self._is_url = False
            self._is_fsspec = False
            self._url_data = ""
        else:
            super().__init__(v, mode=mode, **k)

    @contextmanager
    def open(self, mode: str = "r") -> Iterator[IO]:
        """Return an opened file object for the path."""
        if self.absolute == "-":
            if "r" in mode:
                yield sys.stdin
            elif "w" in mode:
                yield sys.stdout
        else:
            with super().open(mode) as handle:
                yield handle

I am interested in implementing it but maybe I could use a little help.

Should I fork the repo?

mauvilsa commented 5 months ago

I am interested in implementing it but maybe I could use a little help.

Certainly, I can help. Steps would be:

Also before developing, be sure to have a read of https://github.com/omni-us/jsonargparse/blob/main/CONTRIBUTING.rst

mquillot commented 5 months ago

Ok, I coded this new feature. Now, I have a question before sending my pull request.

How can I check if the unit tests cover 100% the code? Is it possible to do it automatically?

I am not sure to know how to update the CHANGELOG file as it refers to different versions etc .. Can you help me with that aspect?

Thank you.

mauvilsa commented 5 months ago

How can I check if the unit tests cover 100% the code? Is it possible to do it automatically?

How to run the coverage is explained at the bottom in CONTRIBUTING.rst. Though, not mentioned there would be how to get a nice html output. Do:

pytest --cov --cov-report=html

Then open in a browser htmlcov/index.html.

I am not sure to know how to update the CHANGELOG file as it refers to different versions etc .. Can you help me with that aspect?

This is a new feature, so add at the top a new section like:

v4.28.0 (2024-03-??)
--------------------

Added
^^^^^
- ...
mquillot commented 5 months ago

It was a little bit complicated to avoid conflict with the following feature: giving "-" to specify that a list of files is given as input. So, I proposed a solution. I hope it will be ok for you.

You will find my PR here: https://github.com/omni-us/jsonargparse/pull/475