Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

DeprecationWarning removed for op-level token_embedders #2955

Merged
merged 8 commits into from Jun 18, 2019
Merged

DeprecationWarning removed for op-level token_embedders #2955

merged 8 commits into from Jun 18, 2019

Conversation

pidugusundeep
Copy link
Contributor

@pidugusundeep pidugusundeep commented Jun 15, 2019 •
edited by matt-gardner

Fixes #2252

@matt-gardner
Copy link
Contributor

Thanks! You need to also remove the check for the DeprecationWarning in the test that's failing.

@pidugusundeep pidugusundeep mentioned this pull request Jun 17, 2019
Closed
@pidugusundeep
Copy link
Contributor Author

Thanks! You need to also remove the check for the DeprecationWarning in the test that's failing.

Got it, Removed the test and added a PR

@matt-gardner
Copy link
Contributor

Sorry I wasn't clear - we still want the test for the old format, because we still handle the old format. We just need to remove the check for the warning from the test, because the warning doesn't appear anymore. So it's just removing what was line 215, and un-indenting the line below it.

@pidugusundeep
Copy link
Contributor Author

Sorry I wasn't clear - we still want the test for the old format, because we still handle the old format. We just need to remove the check for the warning from the test, because the warning doesn't appear anymore. So it's just removing what was line 215, and un-indenting the line below it.

Got it attached a PR

@SundeepPidugu SundeepPidugu mentioned this pull request Jun 18, 2019
Closed
@matt-gardner
Copy link
Contributor

Thanks!

@matt-gardner matt-gardner merged commit 0f1bc3b into allenai:master Jun 18, 2019
reiyw pushed a commit to reiyw/allennlp that referenced this pull request Nov 12, 2019
@pidugusundeep @reiyw
DeprecationWarning removed for op-level token_embedders (allenai#2955)
d7cb0ad 
* DeprecationWarning removed for op-level token_embedders

* removed test for DeprecationWarning

* removed old_embedder in test

* removed unused warning import

* added old_embedded to test

* Update basic_text_field_embedder_test.py

* Update basic_text_field_embedder_test.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matt-gardner matt-gardner matt-gardner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Timeline for deprecating top-level token_embedders?
2 participants
@pidugusundeep @matt-gardner