Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

[CI] Test warnings #980

Merged
merged 32 commits into from
Oct 25, 2019
Merged

[CI] Test warnings #980

merged 32 commits into from
Oct 25, 2019

Conversation

leezu
Copy link
Contributor

@leezu leezu commented Oct 21, 2019 •
edited
Loading

Description

Tell pytest to turn all warnings into errors.
For example, this helps to enforce that we don't run into problems like #978

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

Changes

  • Update gluonnlp.data.UnigramCandidateSampler to make use of MXNet random.uniform_like operator and drop constructor shape argument. Now the candidates_like argument during forward is used to specify shape of to be sampled candidates. This was the originally intended design, but had to wait for MXNet random.uniform_like operator being available in stable MXNet.

cc @dmlc/gluon-nlp-team

@leezu
Fix naming of TestNamedTuple
244c121 
Fixes "PytestCollectionWarning: cannot collect test class 'TestNamedTuple'
because it has a __new__ constructor"
@leezu
Tell pytest to turn all warnings into errors
51f7b8f
@leezu leezu requested a review from a team as a code owner October 21, 2019 18:24
@codecov
Copy link

codecov bot commented Oct 21, 2019 •
edited
Loading

Codecov Report

Merging #980 into master will increase coverage by 8.53%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #980      +/-   ##
==========================================
+ Coverage   81.47%   90.01%   +8.53%     
==========================================
  Files          67       67              
  Lines        6371     6482     +111     
==========================================
+ Hits         5191     5835     +644     
+ Misses       1180      647     -533
Impacted Files Coverage Δ
src/gluonnlp/data/batchify/batchify.py 95.27% <ø> (-0.79%) ⬇️
src/gluonnlp/data/glue.py 98.63% <ø> (+1.81%) ⬆️
src/gluonnlp/data/word_embedding_evaluation.py 92.39% <ø> (+3.38%) ⬆️
src/gluonnlp/data/transforms.py 86% <ø> (ø) ⬆️
src/gluonnlp/vocab/subwords.py 87.23% <100%> (+0.27%) ⬆️
src/gluonnlp/data/candidate_sampler.py 91.83% <100%> (-0.48%) ⬇️
src/gluonnlp/embedding/token_embedding.py 92.01% <100%> (+2.89%) ⬆️
src/gluonnlp/model/attention_cell.py 96.45% <0%> (+2.05%) ⬆️
... and 18 more

leezu
leezu commented Oct 21, 2019
View reviewed changes
tests/unittest/batchify/test_batchify.py
@@ -12,28 +12,28 @@ def test_list():
passthrough = batchify.List()(data)
assert passthrough == data

TestNamedTuple = namedtuple('TestNamedTuple', ['data', 'label'])
MyNamedTuple = namedtuple('MyNamedTuple', ['data', 'label'])
Copy link
Contributor Author

@leezu leezu Oct 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sxjscience FYI

@leezu
Don't register TOY dataset in gluonnlp data registry
70a6bb1 
Fixes "UserWarning: New dataset dataset.TOY registered with name toy isoverriding existing dataset scripts.machine_translation.dataset.TOY"
@leezu
Don't trigger Pad(pad_val=None) warnings in GluonNLP codebase
6f05f28
@leezu
Fix lint
4b498ed
@leezu
Ignore warnings triggered by invalid entries in GlueQQP dev set
6c65ea7
@leezu
Avoid warnings in BERTSPTokenizer
f52f174
@leezu
Fix syntax
1a8282d
@leezu
Fix import
b94e2dd
@leezu
Fix syntax
8569dd2
@leezu
Correct all overwrite=True
401d3c1
@leezu
Ignore corrupted vocab index warning for TOY dataset for now
6c448ba
@leezu
Don't use deprecated mx.Context.default_ctx
e3f8817
@leezu
Remove outdated '-etc-'s from docstring
6e889b7
@leezu
Fix use of current_context()
b6a3d03
@leezu
Use random.uniform_like in UnigramCandidateSampler
7eac3ce 
Fixes 'UserWarning: The 0-th input to HybridBlock is not used by any
computation. Is this intended?'
@leezu
Ignore warnings for FixedBucketSampler
e57f3a8
@leezu
Fix test_intent_slot
b836a44
@leezu
Remove second initialize() in test_big_rnn_model_share_params
6a43a4e
@leezu
Fix lint
063485f
@leezu
Add back two '-etc-'
0128c24
@leezu
Make gluon.utils.download in docstrings deterministic
a80efbd 
By deleting files again
@leezu
Fix UnigramCandidateSampler
382c3e0
@leezu
Fix test_toy
4f06300
@mli
Copy link
Member

mli commented Oct 23, 2019

Job PR-980/14 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/14/index.html

@mli
Copy link
Member

mli commented Oct 24, 2019

Job PR-980/15 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/15/index.html

@mli
Copy link
Member

mli commented Oct 24, 2019

Job PR-980/16 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/16/index.html

@mli
Copy link
Member

mli commented Oct 24, 2019

Job PR-980/18 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/18/index.html

@mli
Copy link
Member

mli commented Oct 24, 2019

Job PR-980/19 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/19/index.html

@mli
Copy link
Member

mli commented Oct 25, 2019

Job PR-980/20 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/20/index.html

@leezu leezu requested a review from sxjscience October 25, 2019 04:54
@leezu leezu added the release focus Progress focus for release label Oct 25, 2019
szha
szha approved these changes Oct 25, 2019
View reviewed changes
Copy link
Member

@szha szha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please document all API changes in the PR comment to be added to release note later.

@leezu
Copy link
Contributor Author

leezu commented Oct 25, 2019

Yes, I'll add them now. Thanks

@mli
Copy link
Member

mli commented Oct 25, 2019

Job PR-980/21 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-980/21/index.html

@leezu leezu merged commit 7c4fd12 into dmlc:master Oct 25, 2019
@leezu leezu deleted the warningserror branch October 25, 2019 20:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@szha szha szha approved these changes

@sxjscience sxjscience Awaiting requested review from sxjscience

Assignees
No one assigned
Labels
API change release focus Progress focus for release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@leezu @mli @szha