djanand commented on PR #1883:
URL:
https://github.com/apache/datafusion-ballista/pull/1883#issuecomment-4764567517
### Design note: why not use `@magic_arguments`?
IPython's `@magic_arguments` / `parse_argstring` is the conventional way
to parse magic options (and would give `--limit=5` and type validation for
free). I deliberately avoided it here:
- This module is written to import and be unit-tested **without IPython**
— it provides stub decorators for the `IPYTHON_AVAILABLE == False` path,
`ipython` is only an optional `[jupyter]` extra (not a base or dev dependency),
and the test suite imports `BallistaMagics` with no IPython installed.
`magic_arguments`/`parse_argstring` are not stubbed and would be imported
unconditionally, breaking that IPython-less path.
- The hand-rolled `line.split()` parsing matches the existing convention
used by the other magics in this file (`%ballista`, `%register`); there is no
`argparse`/`magic_arguments` usage anywhere in the module today.
If the project would prefer the `@magic_arguments` style, that'd be better
done as a separate refactor converting all the magics at once IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
