-
-
Notifications
You must be signed in to change notification settings - Fork 276
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
Organize help options #1540
Organize help options #1540
Conversation
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
@koxudaxi any suggestions or concerns? |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1540 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 11 33 +22
Lines 1020 3621 +2601
Branches 201 841 +640
===========================================
+ Hits 1020 3621 +2601
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@howardj99
Thank you for creating the PR.
I prefer your changes.
I left a few comments.
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
@koxudaxi Should be good to run the checks again whenever you get a chance |
@koxudaxi How would you feel about moving the group/argument definitions into a JSON/YAML file to be read in and used as parameters to Something like {
"general": [
{
"flags": ["--debug"],
"help": "show debug message",
"action": "store_true",
"default": null
},
{
"flags": ["--disable-warnings"],
"help": "disable warnings",
"action": "store_true",
"default": null
}
]
} just as an example. Should make |
It looks good. We lack type-checking and code auto-completion. Still, the benefits will be significant. |
Looks like it would have to be a jinja template or something rather than a static JSON file, since some of the parameters are dynamically generated with list comprehensions or bare types like Full JSON example (invalid){
"base": [
{
"flags": ["--http-headers"],
"nargs": "+",
"metavar": "HTTP_HEADER",
"help": "Set headers in HTTP requests to the remote host. (example: \"Authorization: Basic dXNlcjpwYXNz\")"
},
{
"flags": ["--http-ignore-tls"],
"help": "Disable verification of the remote host's TLS certificate",
"action": "store_true",
"default": null
},
{
"flags": ["--input-file-type"],
"help": "Input file type (default: auto)",
"choices": [i.value for i in InputFileType]
},
{
"flags": ["--input"],
"help": "Input file/directory (default: stdin)"
},
{
"flags": ["--output-model-type"],
"help": "Output model type (default: pydantic.BaseModel)",
"choices": [i.value for i in DataModelType]
},
{
"flags": ["--output"],
"help": "Output file (default: stdout)"
},
{
"flags": ["--url"],
"help": "Input file URL. `--input` is ignored when `--url` is used"
}
],
"typing": [
{
"flags": ["--base-class"],
"help": "Base Class (default: pydantic.BaseModel)",
"type": str
},
{
"flags": ["--enum-field-as-literal"],
"help": "Parse enum field as literal. all: all enum field type are Literal. one: field type is Literal when an enum has only one possible value",
"choices": [lt.value for lt in LiteralType],
"default": null
},
{
"flags": ["--field-constraints"],
"help": "Use field constraints and not con* annotations",
"action": "store_true",
"default": null
},
{
"flags": ["--set-default-enum-member"],
"help": "Set enum members as default values for enum field",
"action": "store_true",
"default": null
},
{
"flags": ["--strict-types"],
"help": "Use strict types",
"choices": [t.value for t in StrictTypes],
"nargs": "+"
},
{
"flags": ["--use-annotated"],
"help": "Use typing.Annotated for Field(). Also, `--field-constraints` option will be enabled.",
"action": "store_true",
"default": null
},
{
"flags": ["--use-generic-container-types"],
"help": "Use generic container types for type hinting (typing.Sequence, typing.Mapping). If `--use-standard-collections` option is set, then import from collections.abc instead of typing",
"action": "store_true",
"default": null
},
{
"flags": ["--use-non-positive-negative-number-constrained-types"],
"help": "Use the Non{Positive,Negative}{FloatInt} types instead of the corresponding con* constrained types.",
"action": "store_true",
"default": null
},
{
"flags": ["--use-one-literal-as-default"],
"help": "Use one literal as default value for one literal field",
"action": "store_true",
"default": null
},
{
"flags": ["--use-standard-collections"],
"help": "Use standard collections for type hinting (list, dict)",
"action": "store_true",
"default": null
},
{
"flags": ["--use-subclass-enum"],
"help": "Define Enum class as subclass with field type when enum has type (int, float, bytes, str)",
"action": "store_true",
"default": null
},
{
"flags": ["--use-union-operator"],
"help": "Use | operator for Union type (PEP 604).",
"action": "store_true",
"default": null
},
{
"flags": ["--use-unique-items-as-set"],
"help": "define field type as `set` when the field attribute has `uniqueItems`",
"action": "store_true",
"default": null
}
],
"field": [
{
"flags": ["--capitalise-enum-members", "--capitalize-enum-members"],
"help": "Capitalize field names on enum",
"action": "store_true",
"default": null
},
{
"flags": ["--empty-enum-field-name"],
"help": "Set field name when enum value is empty (default: `_`)",
"default": null
},
{
"flags": ["--field-extra-keys-without-x-prefix"],
"help": "Add extra keys with `x-` prefix to field parameters. The extra keys are stripped of the `x-` prefix.",
"type": str,
"nargs": "+"
},
{
"flags": ["--field-extra-keys"],
"help": "Add extra keys to field parameters",
"type": str,
"nargs": "+"
},
{
"flags": ["--field-include-all-keys"],
"help": "Add all keys to field parameters",
"action": "store_true",
"default": null
},
{
"flags": ["--force-optional"],
"help": "Force optional for required fields",
"action": "store_true",
"default": null
},
{
"flags": ["--original-field-name-delimiter"],
"help": "Set delimiter to convert to snake case. This option only can be used with --snake-case-field (default: `_` )",
"default": null
},
{
"flags": ["--remove-special-field-name-prefix"],
"help": "Remove field name prefix when first character can't be used as Python field name",
"action": "store_true",
"default": null
},
{
"flags": ["--snake-case-field"],
"help": "Change camel-case field name to snake-case",
"action": "store_true",
"default": null
},
{
"flags": ["--special-field-name-prefix"],
"help": "Set field name prefix when first character can't be used as Python field name (default: `field`)",
"default": null
},
{
"flags": ["--strip-default-none"],
"help": "Strip default None on fields",
"action": "store_true",
"default": null
},
{
"flags": ["--use-default-kwarg"],
"action": "store_true",
"help": "Use `default=` instead of a positional argument for Fields that have default values.",
"default": null
},
{
"flags": ["--use-default"],
"help": "Use default value even if a field is required",
"action": "store_true",
"default": null
},
{
"flags": ["--use-field-description"],
"help": "Use schema description to populate field docstring",
"action": "store_true",
"default": null
}
],
"model": [
{
"flags": ["--allow-extra-fields"],
"help": "Allow to pass extra fields, if this flag is not passed, extra fields are forbidden.",
"action": "store_true",
"default": null
},
{
"flags": ["--allow-population-by-field-name"],
"help": "Allow population by field name",
"action": "store_true",
"default": null
},
{
"flags": ["--class-name"],
"help": "Set class name of root model",
"default": null
},
{
"flags": ["--collapse-root-models"],
"action": "store_true",
"default": null,
"help": "Models generated with a root-type field will be mergedinto the models using that root-type model"
},
{
"flags": ["--disable-appending-item-suffix"],
"help": "Disable appending `Item` suffix to model name in an array",
"action": "store_true",
"default": null
},
{
"flags": ["--disable-timestamp"],
"help": "Disable timestamp on file headers",
"action": "store_true",
"default": null
},
{
"flags": ["--enable-faux-immutability"],
"help": "Enable faux immutability",
"action": "store_true",
"default": null
},
{
"flags": ["--enable-version-header"],
"help": "Enable package version on file headers",
"action": "store_true",
"default": null
},
{
"flags": ["--keep-model-order"],
"help": "Keep generated models' order",
"action": "store_true",
"default": null
},
{
"flags": ["--reuse-model"],
"help": "Re-use models on the field when a module has the model with the same content",
"action": "store_true",
"default": null
},
{
"flags": ["--target-python-version"],
"help": "target python version (default: 3.7)",
"choices": [v.value for v in PythonVersion]
},
{
"flags": ["--use-schema-description"],
"help": "Use schema description to populate class docstring",
"action": "store_true",
"default": null
},
{
"flags": ["--use-title-as-name"],
"help": "use titles as class names of models",
"action": "store_true",
"default": null
}
],
"template": [
{
"flags": ["--aliases"],
"help": "Alias mapping file",
"type": FileType("rt")
},
{
"flags": ["--custom-file-header-path"],
"help": "Custom file header file path",
"default": null,
"type": str
},
{
"flags": ["--custom-file-header"],
"help": "Custom file header",
"type": str,
"default": null
},
{
"flags": ["--custom-template-dir"],
"help": "Custom template directory",
"type": str
},
{
"flags": ["--encoding"],
"help": f"The encoding of input and output (default: {DEFAULT_ENCODING})",
"default": null
},
{
"flags": ["--extra-template-data"],
"help": "Extra template data",
"type": FileType("rt")
},
{
"flags": ["--use-double-quotes"],
"action": "store_true",
"default": null,
"help": "Model generated with double quotes. Single quotes or your black config skip_string_normalization value will be used without this option."
},
{
"flags": ["--wrap-string-literal"],
"help": "Wrap string literal by using black `experimental-string-processing` option (require black 20.8b0 or later)",
"action": "store_true",
"default": null
}
],
"openapi": [
{
"flags": ["--openapi-scopes"],
"help": "Scopes of OpenAPI model generation (default: schemas)",
"choices": [o.value for o in OpenAPIScope],
"nargs": "+",
"default": null
},
{
"flags": ["--strict-nullable"],
"help": "Treat default field as a non-nullable field (Only OpenAPI)",
"action": "store_true",
"default": null
},
{
"flags": ["--use-operation-id-as-name"],
"help": "use operation id of OpenAPI as class names of models",
"action": "store_true",
"default": null
},
{
"flags": ["--validation"],
"help": "Enable validation (Only OpenAPI)",
"action": "store_true",
"default": null
}
],
"general": [
{
"flags": ["--debug"],
"help": "show debug message",
"action": "store_true",
"default": null
},
{
"flags": ["--disable-warnings"],
"help": "disable warnings",
"action": "store_true",
"default": null
},
{
"flags": ["--no-color"],
"help": "disable colorized output",
"action": "store_true"
},
{
"flags": ["--version"],
"help": "show version",
"action": "store_true"
},
{
"flags": ["-h", "--help"],
"action": "help",
"default": "==SUPPRESS==",
"help": "show this help message and exit"
}
]
} |
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
@koxudaxi instead of the JSON file approach, I figured if creating a new file anyway, might as well be a python module 😛 Are you okay with this change? I also have an alternative implementation if you prefer, where the groups/args are defined in a arg_groups: Dict[str, List[Dict[str, Any]]] = {
'Options': [
{
'flags': ['--http-headers'],
'nargs': '+',
'metavar': 'HTTP_HEADER',
'help': 'Set headers in HTTP requests to the remote host. (example: "Authorization: Basic dXNlcjpwYXNz")',
},
],
'Typing customization': [
{
'flags': ['--base-class'],
'help': 'Base Class (default: pydantic.BaseModel)',
'type': str,
},
# ...
],
# ...
}
for group_name, args in arg_groups.items():
group = arg_parser.add_argument_group(group_name)
for arg in args:
flags, kwargs = arg.pop('flags'), arg
group.add_argument(*flags, **kwargs) |
@howardj99 I agree with the approach. arg_groups: Dict[str, List[Dict[str, Any]]] But, Can we use |
You're saying you'd rather go with that approach instead? Right now it's just a bunch of static calls to |
I would like to check runtime for typechecking or unittest to make sure each type and the key is correct, even if it is exaggerated. |
Signed-off-by: Jonathan Howard <howard.jonathan.21@gmail.com>
@koxudaxi please take a look at this commit and let me know if you prefer it the new way or the old way |
Looks like that broke everything. Weird that it was passing unit tests locally. Would you be ok with the previous implementation, without the dict or JSON data? |
This reverts commit 86b4ca7.
I checked your changes.
We will probably get the chance to improve the part of the argument next time. Thank you very much!! |
Address #1539
--capitalize-enum-members
as alternative spelling to existing--capitalise-enum-members
optionsHelp output with this change applied: