Skip to content

Moved some functions out of run_all_in_graph_and_eager_mode in attention_wrapper_test.py. #1639

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

Merged
merged 2 commits into from
Apr 17, 2020
Merged

Moved some functions out of run_all_in_graph_and_eager_mode in attention_wrapper_test.py. #1639

merged 2 commits into from
Apr 17, 2020

Conversation

gabrieldemarmiesse
Copy link
Member

No description provided.

Copy link
Member

@autoih autoih left a comment

Choose a reason for hiding this comment

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

Thanks @gabrieldemarmiesse. LGTM

@gabrieldemarmiesse gabrieldemarmiesse changed the title Moved some functions out of run_all_in_graph_and_eager_mode. Moved some functions out of run_all_in_graph_and_eager_mode in attention_wrapper_test.py. Apr 13, 2020
Copy link
Member

@qlzh727 qlzh727 left a comment

Choose a reason for hiding this comment

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

The indent seems to be quite weird in this file, are we using 4 space or 2 space?

Also, I haven't follow the change history closely, but currently there is a nest structure of test classes in this file, which is bit anti-pattern. Is that expected?

@gabrieldemarmiesse
Copy link
Member Author

gabrieldemarmiesse commented Apr 13, 2020

The indent seems to be quite weird in this file, are we using 4 space or 2 space?

I'm not sure I follow you, it's 4 spaces, it's enforced with black.

Also for the background, sorry, I should have been more explicit, see #1328

We want to get rid of the decorator that traces test functions (run_all_in_graph_and_eager_mode).

@autoih
Copy link
Member

autoih commented Apr 13, 2020

The indent seems to be quite weird in this file, are we using 4 space or 2 space?

I think in TensorFlow using 2 space, but here is with black, which is 4 space.

@AakashKumarNain AakashKumarNain merged commit e92a36a into tensorflow:master Apr 17, 2020
jrruijli pushed a commit to jrruijli/addons that referenced this pull request Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants