-
Notifications
You must be signed in to change notification settings - Fork 8
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
Get rid of warnings when running tests #180
base: master
Are you sure you want to change the base?
Conversation
@@ -168,6 +172,7 @@ def test_display_pca(self, dnn_class): | |||
|
|||
def test_display_tsne(self, dnn_class): | |||
"""Test t-SNE displays and saves.""" | |||
warnings.filterwarnings('ignore', category=PendingDeprecationWarning) |
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.
can we instead look for something to replace this that's not pending deprecation? we may find ourselves in trouble in the future and may as well figure out how to solve it now.
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.
My comment above is still relevant ^ I believe.
Thanks Divya! :) I’m happy to answer any questions during the process.
On Jun 25, 2019, at 12:58 PM, divyachandran-ds <notifications@github.com<mailto:notifications@github.com>> wrote:
@nirtiac<https://github.com/nirtiac> @rfratila<https://github.com/rfratila>
I will work on all the above suggestions and get back to you.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/Aifred-Health/Vulcan/pull/180?email_source=notifications&email_token=AB6LRLJEPLXSWMGSNEXANITP4JFBXA5CNFSM4H2U7ZL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYQ45AI#issuecomment-505532033>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AB6LRLITSSGODUEE226ORG3P4JFBXANCNFSM4H2U7ZLQ>.
|
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.
Hi Divya,
Can you explain the benefit of
'''elif interactive is True:
plt.draw()
plt.pause(1e-17)'''
Over what was already there? I'm afraid I've forgotten by then.
Also, if the solution for most of the warnings is to have an updated sklearn/numpy/whatever package, can you update the requirements.txt to include the required package versions?
It was for the interactive display part, it was already added in some other
plots so added it similarly with similar time duration
I will update the requirements
…On Thu, Aug 8, 2019 at 4:05 PM Caitrin Armstrong ***@***.***> wrote:
***@***.**** commented on this pull request.
Hi Divya,
Can you explain the benefit of
plt.draw()
plt.pause(1e-17)```
Over what was already there? I'm afraid I've forgotten by then.
Also, if the solution for most of the warnings is to have an updated sklearn/numpy/whatever package, can you update the requirements.txt to include the required package versions?
------------------------------
In vulcanai/tests/models/test_metrics.py
<https://github.com/Aifred-Health/Vulcan/pull/180#discussion_r312219152>:
> @@ -294,13 +293,11 @@ def test_get_auc(self, metrics):
def test_run_test(self, metrics, cnn_class):
"""Test that run_test returns values as expected."""
+ warnings.filterwarnings("ignore", category=sklearn.exceptions.UndefinedMetricWarning)
this is just to deal with the sklearn deprecated stuff
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<https://github.com/Aifred-Health/Vulcan/pull/180?email_source=notifications&email_token=AKYUR5C2QPD7BZCLC7H5KTDQDR4BVA5CNFSM4H2U7ZL2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBBLJ6I#pullrequestreview-272807161>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKYUR5FUBQJC4EA5SNO454LQDR4BVANCNFSM4H2U7ZLQ>
.
|
@divyachandran-ds can you update the requirements as discussed above? ^ |
Hi Caitrin
I have updated the requirements
Regards
Divya.
… On Aug 27, 2019, at 10:20 AM, Caitrin Armstrong ***@***.***> wrote:
@divyachandran-ds <https://github.com/divyachandran-ds> can you update the requirements as discussed above? ^
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <https://github.com/Aifred-Health/Vulcan/pull/180?email_source=notifications&email_token=AKYUR5GINMYJYESL5TNFGGLQGUZ3FA5CNFSM4H2U7ZL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5H4ZQY#issuecomment-525323459>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKYUR5HSUOIG3LP7NXC4YTTQGUZ3FANCNFSM4H2U7ZLQ>.
|
*cnn_class_binary.in_dim))) | ||
test_target = torch.LongTensor(np.random.randint(0, 2, | ||
size=num_items)) | ||
test_input = torch.Tensor(np.random.randint(0, 2,size=(num_items,*cnn_class_binary.in_dim))) |
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.
please make sure that every line is within the limit
No description provided.