Bookworm-project / BookwormDB

Tools for text tokenization and encoding
MIT License
84 stars 12 forks source link

Constrain bookworm prep options #130

Closed organisciak closed 7 years ago

organisciak commented 7 years ago

It's probably better to explicitly state which arguments are allowed for bookworm prep {goal} with argparse subparsers. Since you're calling getattr(self)(args.goal), this would help avoid bugs and unsafe operations. It also improves the --help documentation.

@bmschmidt, if you agree, I can check something in. My modification is similar to the following, using the function's docstring for documenting the arg:

extensions_subparsers = extensions_parser.add_subparsers(title="goal",help="The name of the target.", dest="goal")  

for prep_arg in ['preDatabaseMetadata', 'text_id_database', 'catalog_metadata', 'database_metadata', 'database_wordcounts']:
    extensions_subparsers.add_parser(prep_arg, help=getattr(BookwormManager, prep_arg).__doc__)

image

bmschmidt commented 7 years ago

This would be enthusiastically accepted.

It may break some of my code, though probably not anyone else's.

One other method besides those listed should be included:, bookworm prep guessAtFieldDescriptions, which creates a field_descriptions.json file according to a few rules of thumb if one does not already exist.

bmschmidt commented 7 years ago

I pushed a little to the docstring on guessAtFieldDescriptions.

At some point the Makefile should be retired entirely and with it the bookworm build formula, but I'm not honestly sure when I'll get around to doing that. But if you encounter anything in the Hathi construction that seems to require bookworm build rather than bookworm prep, it's likely that it would be a useful candidate to get broken out as a new bookworm prep argument.