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

show help for no-arg processor calls, fix #562, fix #274 #586

Merged
merged 3 commits into from
Sep 22, 2020
Merged

Conversation

kba
Copy link
Member

@kba kba commented Sep 2, 2020

This is a surprisingly tricky-to-implement problem: We want processor calls without any arguments to print the help screen and exit with code 1, i.e. if sys.argv[2:] == [].

It really isn't, it was just hard to test because sys.argv contained the arguments to pytest, not to the CLI. Turns out, sys.argv can be overridden at runtime for the tests and checking not sys.argv[1:] was enough

However, I could not find a way to get the un-procesed command line arguments to click. `get_current_context()` does have an `args` property but that only contains the remaining positional arguments after parsing.

If anyone knows a better way to get the click equivalent of sys.argv[2:], I'd be interested!

For now, I've disabled the defaults for input_file_grp/output_file_grp (as proposed in #274) and check for all relevant CLI options one-by-one:

    if not (dump_json or help or version or overwrite) and \
            mets == 'mets.xml' and \
            not (kwargs['input_file_grp'] or kwargs['output_file_grp']):
        processorClass(workspace=None, show_help=True)
        sys.exit(1)

As I said before, I would prefer a more robust solution. The clause above will also trigger for processors being called with --mets mets.xml but neither input nor output file group (such as the ocrd-sanitize processor...).

@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2020

Codecov Report

Merging #586 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #586   +/-   ##
=======================================
  Coverage   84.87%   84.87%           
=======================================
  Files          52       52           
  Lines        2890     2890           
  Branches      564      564           
=======================================
  Hits         2453     2453           
  Misses        328      328           
  Partials      109      109           
Impacted Files Coverage Δ
ocrd/ocrd/decorators/__init__.py 88.57% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9c3d32...ae20fdb. Read the comment docs.

@kba kba requested a review from bertsky September 22, 2020 10:35
Copy link
Collaborator

@bertsky bertsky left a comment

Choose a reason for hiding this comment

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

Energize!

@kba kba merged commit cb5b586 into master Sep 22, 2020
@kba kba deleted the no-args-help branch September 22, 2020 10:59
@mikegerber
Copy link
Contributor

mikegerber commented Sep 25, 2020

This sometimes breaks running a test using click.testing.CliRunner, e.g.:

   # Run through the OCR-D interface                                                                                   
    with working_directory(str(test_workspace_dir)):                                                                    
        runner = CliRunner()                                                                                            
        result = runner.invoke(ocrd_dinglehopper, [                                                                                                          
            '-m', 'mets.xml',                                                                                           
            '-I', 'OCR-D-GT-PAGE,OCR-D-OCR-CALAMARI',                                                                   
            '-O', 'OCR-D-OCR-CALAMARI-EVAL'                                                                             
        ])                                                                                                              
    print(result.output, file=sys.stderr)                                                                               
    assert result.exit_code == 0

The call/invoke prints help and exits with 1. (Funny, it's also a Heisenbug, it doesn't break when I run the test alone, but does when I run all the tests.)

Edit: I worked around it by setting sys.argv[1:] manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants