-
Notifications
You must be signed in to change notification settings - Fork 2
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
Pass mypy and link issues #42
Conversation
43fdc58
to
f84f255
Compare
jaraco/mongodb/move-gridfs.py
Outdated
@@ -120,7 +122,7 @@ def run( | |||
dest_gfs: helper.connect_gridfs, | |||
include: (str, "a filter of files (regex) to include") = None, # noqa: F722 | |||
delete: (bool, "delete files after moving") = False, # noqa: F722 | |||
limit: int = None, | |||
limit: int | None = None, |
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.
Not your fault, but I think this function may be broken. It looks like someone (me) started working in converting this routine to autocommand but never finished. Indeed, it was in 8836704, not too long ago. The reason the incomplete approach didn't lead to failures is because tests for that module are disabled.
All that to say, the broken command is not your fault, but we are going to want to add the autocommand decorator, and I'm uncertain if it supports this syntax.
Let me fix the usage separately and then we can rebase/merge to ensure this also works.
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.
I confirmed, when configured correctly, it's not allowed to have a union of types:
jaraco.mongodb main 🐚 .tox/py/bin/python -m jaraco.mongodb.move-gridfs --help
Traceback (most recent call last):
File "<frozen runpy>", line 198, in _run_module_as_main
File "<frozen runpy>", line 88, in _run_code
File "/Users/jaraco/code/jaraco/jaraco.mongodb/jaraco/mongodb/move-gridfs.py", line 118, in <module>
@autocommand.autocommand(__name__)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jaraco/.local/pip-run/autocommand/autocommand.py", line 57, in autocommand_decorator
func = autoparse(
^^^^^^^^^^
File "/Users/jaraco/.local/pip-run/autocommand/autoparse.py", line 284, in autoparse
parser = make_parser(
^^^^^^^^^^^^
File "/Users/jaraco/.local/pip-run/autocommand/autoparse.py", line 207, in make_parser
_add_arguments(param, parser, used_char_args, add_nos)
File "/Users/jaraco/.local/pip-run/autocommand/autoparse.py", line 107, in _add_arguments
arg_type, description = _get_type_description(param.annotation)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jaraco/.local/pip-run/autocommand/autoparse.py", line 80, in _get_type_description
raise AnnotationError(annotation)
autocommand.autoparse.AnnotationError: int | None
Let's just suppress the type check for now, as I'm migrating my projects from the abandoned autocommand to typer, which has better support for type annotations.
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.
I figured autocommand was doing something special with annotations, didn't realize this function was just not decorated properly.
I'm guessing limit: int = 0
might be more accurate. But a suppression comment with a link will do.
Upstream ref: Lucretiel/autocommand#30
790e62a
to
64aa245
Compare
64aa245
to
9fdbf11
Compare
Ref: jaraco/skeleton#143
Upstream issues:
py.typed
jaraco.services#5py.typed
portend#17