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

Fixes to work with TensorFlow 1.2 #254

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

darrengarvey
Copy link

Some minor, albeit ugly, fixes to support TF1.2 - tested against 1.2.0-rc1.

I've checked these don't break the tests for the current TF1.1 version.

Fixes the following error when running the tests on Python 3.

    Traceback (most recent call last):
      File "seq2seq/seq2seq/test/hooks_test.py", line
      55, in test_begin
          self.assertEqual(file_contents.decode(), ...)
          AttributeError: 'str' object has no attribute 'decode'
distutils.version isn't available on the CI versions, so change the test
to not rely explicitly on the version of TensorFlow installed.
@darrengarvey
Copy link
Author

Phew. A couple of failed attempts against CircleCI, but hopefully useful now it works. :)

Use of `tf.name_scope` and `tf.variable_scope` cause pylint errors on TF1.2
due to pylint not fully understanding the `tf_contextlib.contextmanager`
decorators.
Following some changes in TF to the `LazyLoader` [1], pylint complains
about not being able to find some imports under `tf.contrib`. This looks
like a pylint issue, emitting errors like:

    ************* Module seq2seq.encoders.rnn_encoder
    E: 24, 0: No name 'rnn' in module 'LazyLoader' (no-name-in-module)
    ************* Module seq2seq.data.input_pipeline
    E: 32, 0: No name 'slim' in module 'LazyLoader' (no-name-in-module)

[1] tensorflow/tensorflow@95c5d7e
@darrengarvey
Copy link
Author

Not sure what the deal is about author consent. I err, consent.

@gibrown
Copy link

gibrown commented Jul 11, 2017

FYI, this seems to work for me when getting the simple pipeline test to run python -m unittest seq2seq.test.pipeline_test

@darrengarvey
Copy link
Author

I think I need to say "I consent" again.

@melvinma
Copy link

What is the plan for this fork? When will it be released? Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants