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

Have a skeleton.py file like in PyScaffold #55

Open
FlorianWilhelm opened this issue Jun 19, 2022 · 4 comments
Open

Have a skeleton.py file like in PyScaffold #55

FlorianWilhelm opened this issue Jun 19, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@FlorianWilhelm
Copy link
Member

Describe your use-case

@sgbaird motivated that having a skeleton.py might be useful for his data science project setup. [#642]

Describe the solution you would like to see for your use-case

Just do not automatically activate the --no-skeleton extension in this extension.

@FlorianWilhelm FlorianWilhelm self-assigned this Jun 19, 2022
@FlorianWilhelm FlorianWilhelm added enhancement New feature or request good first issue Good for newcomers labels Jun 19, 2022
@FlorianWilhelm FlorianWilhelm added this to the v0.8 milestone Jun 19, 2022
@sgbaird
Copy link
Contributor

sgbaird commented Aug 18, 2022

Do you think the train_model.py (e.g. here) could be placed into src to replace the skeleton.py file? It was suggested to me by a collaborator (@kjappelbaum) that I use click and he made a PR to convert the skeleton.py CLI (which I had renamed to core.py) to one using click (moved to cli.py).

Or maybe cli.py could be modified (stripped of xtal2png-specific content) and used as a template.

@sgbaird
Copy link
Contributor

sgbaird commented Aug 18, 2022

I took a brief pass at eliminating some of the xtal2png-specific content, though I think it could be made leaner. For example, whether to let some of the file errors happen naturally at the os package level or whether some of the helper functions should be integrated into the cli function itself. I'm neither attached to a specific configuration nor very familiar with click, so I'll have to defer on some of these.

cli.py

import logging
import os
from glob import glob

import click
from click._compat import get_text_stderr
from click.exceptions import UsageError
from click.utils import echo

from self_driving_lab_demo import __version__
from self_driving_lab_demo.core import _logger, fib, setup_logging


def _show_usage_error(self, file=None):
    if file is None:
        file = get_text_stderr()
    color = None

    echo("Error: %s" % self.format_message(), file=file, color=color)
    if self.ctx is not None:
        color = self.ctx.color
        echo("\n\n" + self.ctx.get_help() + "\n", file=file, color=color)


UsageError.show = _show_usage_error  # type: ignore


def check_save_dir(save_dir):
    if save_dir is None:
        raise UsageError("Please specify a path to a directory to save the PNG files.")


def check_path(path, extension):
    if path is None:
        raise UsageError(
            f"Please specify a path to a {extension} file or "
            f"directory containing {extension} files."
        )


def check_files(path, extension):
    if os.path.isdir(path):
        files = glob(os.path.join(path, f"*.{extension}"))
        if not files:
            raise UsageError(f"No {extension.upper()} files found in directory: {path}")
    elif os.path.isfile(path):
        if not path.endswith(f".{extension}"):
            raise UsageError(f"File must have .{extension} extension: {path}")
        files = [path]

    return files


@click.command("cli")
@click.option("--version", is_flag=True, help="Show version.")
@click.option(
    "--path",
    "-p",
    type=click.Path(
        exists=True,
        dir_okay=True,
        file_okay=True,
        readable=True,
    ),
    help="Input filepath",
)
@click.option(
    "--save-dir",
    "-s",
    type=click.Path(exists=False),
    help="Directory in which to save the output file(s).",
)
@click.option("--encode", "runtype", flag_value="encode", help="Encode values.")
@click.option("--decode", "runtype", flag_value="decode", help="Decode values.")
@click.option("--verbose", "-v", help="Set loglevel to INFO.")
@click.option("--very-verbose", "-vv", help="Set loglevel to INFO.")
@click.option(
    "--fibonacci-input",
    "-n",
    type=int,
    default=3,
    help="integer input (n) to calculate the n-th Fibonnaci number",
)
@click.pass_context
def cli(ctx, version, path, save_dir, runtype, verbose, very_verbose, n):
    """
    ${qual_package} command line interface.
    """
    if version:
        click.echo("${qual_package} version: {}".format(__version__))
        return
    if verbose:
        setup_logging(loglevel=logging.INFO)
    if very_verbose:
        setup_logging(loglevel=logging.DEBUG)

    if runtype not in ["encode", "decode"]:
        raise UsageError("Please specify --encode or --decode.")

    _logger.debug("Some message to be displayed when using --very-verbose flag")

    # the core functionality
    fib(n)

    # optionally could read from an input file located at ``path`` and output a file to
    # a ``save_dir``
    path
    save_dir

    click.echo(ctx.get_help())


if __name__ == "__main__":
    cli()

@FlorianWilhelm
Copy link
Member Author

Hi @sgbaird, thanks for your comments. It makes more sense, and is also more of a best practice, to have a cli.py within the actual package, i.e. within src/pkg_name. One could move as you suggested train_model.py to cli.py with the corresponding notes as in skeleton.py, e.g. like the console_scripts settings in setup.cfg. One could do this by default and additionally provide a no-cli command-line-option if some users don't want it.

The script you provided above is a bit complicated and outdated for an example, to be honest. For instance:

@click.option("-v", "--verbose", "log_level", flag_value=logging.INFO)
@click.option("-vv", "--very-verbose", "log_level", flag_value=logging.DEBUG)
def main(cfg_path: Path, log_level: int):

allows you to have the log level directed in log_level so you can do then just:

  logging.basicConfig(
      stream=sys.stdout,
      level=log_level,
      datefmt="%Y-%m-%d %H:%M",
      format="%(asctime)s - %(name)s - %(levelname)s - %(message)s",
  )

and save all the ifs in the example code. Also consider using Python's pathlib library instead of os.path. This also makes the code more robust and even simpler.

I even wonder if one should nowadays even recommend typer instead of click as it uses type-hinting and makes writing a CLI even simpler than click.

Would you want to work on a PR? I can provide coaching if you want to but have no time myself to implement it right now.

@sgbaird
Copy link
Contributor

sgbaird commented Aug 20, 2022

@FlorianWilhelm I think I'll look into typer! Happy to work on a PR, but I might wait until the next time I'm implementing a CI for one of my projects.

sgbaird added a commit to sgbaird/pyscaffoldext-dsproject that referenced this issue Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants