Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Lowercase tf.print when print is a function? #18053

Closed
girving opened this issue Mar 28, 2018 · 10 comments
Closed

Lowercase tf.print when print is a function? #18053

girving opened this issue Mar 28, 2018 · 10 comments
Assignees
@tomerk
Labels
type:feature Feature requests

Comments

@girving
Copy link
Contributor

girving commented Mar 28, 2018

System information

  • Have I written custom code (as opposed to using a stock example script provided in TensorFlow): n/a
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): n/a
  • TensorFlow installed from (source or binary): n/a
  • TensorFlow version (use command below): n/a
  • Python version: n/a
  • Bazel version (if compiling from source): n/a
  • GCC/Compiler version (if compiling from source): n/a
  • CUDA/cuDNN version: n/a
  • GPU model and memory: n/a
  • Exact command to reproduce: n/a

Describe the problem

Since TensorFlow must be compatible with Python 2.7, tf.Print is uppercase. However, tf.print would work fine for Python 3 users and Python 2 users with from __future__ import print_function? There's no difficulty adding this to the source, since all TensorFlow source files have from __future__ import print_function, and it wouldn't interfere with any other 2.7 users.

Objections to me adding?
@asimshankar asimshankar added the API review API Review label Mar 30, 2018
@asimshankar
Copy link
Contributor

asimshankar commented Apr 2, 2018 •
edited

For API addition: Looks good, go ahead :)

@asimshankar asimshankar added stat:contribution welcome Status - Contributions welcome type:feature Feature requests and removed API review API Review labels Apr 2, 2018
@girving
Copy link
Contributor Author

girving commented Apr 3, 2018

Will add once I can build TensorFlow again: looks like it's broken after an Xcode upgrade. :/

ERROR: /private/var/tmp/_bazel_irving/d4bad52af7e8ccc6c91f211db4181990/external/flatbuffers/BUILD:52:1: undeclared
this rule is missing dependency declarations for the following files included by 'external/flatbuffers/src/util.cp
  '/Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include/stdarg.h'
  '/Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include/stddef.h'
  '/Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include/__stddef_max_align_t.h'
  '/Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include/stdint.h'
  '/Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include/limits.h'
Target //tensorflow/tools/pip_package:build_pip_package failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 7.703s, Critical Path: 1.21s
FAILED: Build did NOT complete successfully

girving added a commit to girving/tensorflow that referenced this issue May 11, 2018
@girving
Add tf.print an an alias for tf.Print
750328f 
Users with Python 3 or `from __future__ import print_function` can now
use lowercase `tf.print`.

Fixes tensorflow#18053.
@girving girving mentioned this issue May 11, 2018
Merged
girving added a commit to girving/tensorflow that referenced this issue May 11, 2018
@girving
Add tf.print an an alias for tf.Print
b251176 
Users with Python 3 or `from __future__ import print_function` can now
use lowercase `tf.print`.

Fixes tensorflow#18053.
girving added a commit to girving/tensorflow that referenced this issue May 15, 2018
@girving
Add tf.print an an alias for tf.Print
85e69b5 
Users with Python 3 or `from __future__ import print_function` can now
use lowercase `tf.print`.  `create_python_api.py` needed some adjustment
to ensure that `print_function` doesn't appear as part of the API.

Fixes tensorflow#18053.
girving added a commit to girving/tensorflow that referenced this issue May 15, 2018
@girving
Add tf.print an an alias for tf.Print
45d714a 
Users with Python 3 or `from __future__ import print_function` can now
use lowercase `tf.print`.  `create_python_api.py` needed some adjustment
to ensure that `print_function` doesn't appear as part of the API.

Fixes tensorflow#18053.
rmlarsen pushed a commit that referenced this issue May 17, 2018
@girving @rmlarsen
Add tf.print an an alias for tf.Print (#19234)
9b41e51 
Users with Python 3 or `from __future__ import print_function` can now
use lowercase `tf.print`.  `create_python_api.py` needed some adjustment
to ensure that `print_function` doesn't appear as part of the API.

Fixes #18053.
@raliste
Copy link

raliste commented May 23, 2018

@girving did you find a fix or workaround for the issue regarding the Xcode upgrade? I'm getting the same error.

@girving
Copy link
Contributor Author

girving commented May 23, 2018

@raliste I think eventually all it took was upgrading Bazel viabrew upgrade.

@raliste
Copy link

raliste commented May 23, 2018

Thank you @girving, that did solve the issue.

@asimshankar
Copy link
Contributor

@girving : There are a few other improvements to tf.Print that we were thinking of folding into tf.print (so that it appears and acts a little more like print). So, we're considering undoing this export for the 1.10 release (which will be cut on July 12) and then restoring it for the next release.

Please speak up loudly if you object :)

@asimshankar asimshankar reopened this Jul 4, 2018
@asimshankar asimshankar self-assigned this Jul 4, 2018
@asimshankar asimshankar removed the stat:contribution welcome Status - Contributions welcome label Jul 4, 2018
@girving
Copy link
Contributor Author

girving commented Jul 4, 2018

So the idea is that tf.Print and tf.print would have incompatible behavior? This seems like a bad idea to me if so, but I don't object strongly enough to work to prevent the mistake.

Note that tf.print is accessible only from Python 3 or Python 2.7 with from __future__ import print_function. Making functionality available only with that constraint and making the natural alias do something different would be confusing.

If I've misunderstood and they'll have the same behavior, then I'm not unsure why it needs to be removed and then added back.

@girving
Copy link
Contributor Author

girving commented Jul 4, 2018

Discussed with Martin in person. I'm fine with the change. Amused, even.

tensorflow-copybara pushed a commit that referenced this issue Jul 10, 2018
@asimshankar @tensorflower-gardener
Temporarily unexport tf.print (alias for tf.Print).
42e8b4f 
See #18053

PiperOrigin-RevId: 203975427
@asimshankar asimshankar assigned tomerk and unassigned asimshankar Jul 18, 2018
@tensorflowbutler
Copy link
Member

Nagging Assignee @tomerk: It has been 74 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@tomerk
Copy link

tomerk commented Oct 2, 2018

tf.print has now been added as an operator.

@tomerk tomerk closed this as completed Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tomerk tomerk

Labels
type:feature Feature requests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@asimshankar @girving @raliste @tomerk @tensorflowbutler