-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PYTHON-4480 Deprecate create=True for Collection #1659
Conversation
@@ -197,12 +197,6 @@ def test_create(self): | |||
lambda: "create_test_no_wc" in db.list_collection_names(), | |||
"create create_test_no_wc collection", | |||
) | |||
db.create_test_no_wc.drop() |
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.
We should keep this and catch the warning.
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.
Correct, when we deprecate something we keep the tests and also add an extra assertion to ensure the warning is actually raised.
pymongo/asynchronous/collection.py
Outdated
@@ -248,6 +249,11 @@ def __init__( | |||
|
|||
if create or kwargs: | |||
if _IS_SYNC: | |||
warnings.warn( | |||
"The `create` and `kwargs` arguments are deprecated and will be removed in PyMongo 5.0", |
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 would add "arguments to Collection are..."
@@ -197,12 +197,6 @@ def test_create(self): | |||
lambda: "create_test_no_wc" in db.list_collection_names(), | |||
"create create_test_no_wc collection", | |||
) | |||
db.create_test_no_wc.drop() |
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.
Correct, when we deprecate something we keep the tests and also add an extra assertion to ensure the warning is actually raised.
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.
LGTM!
No description provided.