add travis stuff #3
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3 +/- ##
=========================================
Coverage ? 61.93%
=========================================
Files ? 35
Lines ? 1395
Branches ? 0
=========================================
Hits ? 864
Misses ? 531
Partials ? 0Continue to review full report at Codecov.
|
|
@matt-gardner I chose to just disable the |
allennlp/common/tee_logger.py
Outdated
| @@ -24,7 +24,7 @@ def write(self, message): | |||
| # correctly, so we'll just make sure that each batch shows up on its one line. | |||
| if '\x08' in message: | |||
| message = message.replace('\x08', '') | |||
| if len(message) == 0 or message[-1] != '\n': | |||
| if message or message[-1] != '\n': | |||
There was a problem hiding this comment.
I think this one should be if not message or message[-1] != '\n':.
allennlp/data/vocabulary.py
Outdated
| @@ -210,20 +210,18 @@ def add_token_to_namespace(self, token: str, namespace: str='tokens') -> int: | |||
| self._token_to_index[namespace][token] = index | |||
| self._index_to_token[namespace][index] = token | |||
| return index | |||
| else: | |||
| return self._token_to_index[namespace][token] | |||
| return self._token_to_index[namespace][token] | |||
There was a problem hiding this comment.
I think I disagree with pylint on the readability of this particular error message, and I'm tempted to disable it. It's not a strong opinion, though - what do you think?
There was a problem hiding this comment.
yep agreed, i'll disable it.
There was a problem hiding this comment.
You should disable it in .pylintrc, not inline, by the way, in case you didn't know you could do that.
| chmod +x miniconda.sh && ./miniconda.sh -b -f | ||
| conda update --yes conda | ||
| # If we are running pylint, use Python 3.5.2 due to | ||
| # bug in pylint. https://github.com/PyCQA/pylint/issues/1295 |
There was a problem hiding this comment.
hope you don't mind me interjecting, but this issue is fixed so you should be able to just use python 3.5.3... (no need to create two envs). I could be wrong, though.
There was a problem hiding this comment.
@nelson-liu, your comments are always welcome. Thanks for the reminder - we just upgraded pylint, so yeah, we should have updated that too.
Pull from allennlp master
No description provided.