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.

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

Fix: Can't set None on TextVectorization layer's split parameter problem #36103

Closed
wants to merge 7 commits into from

Conversation

rushabh-v
Copy link
Contributor

resolves: #36071

@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label Jan 21, 2020
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@rushabh-v
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@rushabh-v rushabh-v requested a review from csuter January 21, 2020 17:06
@csuter csuter removed their request for review January 21, 2020 19:27
@rthadur rthadur self-assigned this Jan 21, 2020
@rthadur rthadur added prtype:bugfix PR to fix a bug comp:keras Keras related issues labels Jan 21, 2020
@rushabh-v
Copy link
Contributor Author

Can someone review this PR, please?

tanzhenyu
tanzhenyu previously approved these changes Jan 31, 2020
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jan 31, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 31, 2020
@tensorflow-bot tensorflow-bot bot removed the ready to pull PR ready for merge process label Jan 31, 2020
@rushabh-v
Copy link
Contributor Author

rushabh-v commented Jan 31, 2020

@tanzhenyu Thanks for the approval. I actually forgot to put self argument in the test. I have pushed one more commit. Can you review it again, please?

tanzhenyu
tanzhenyu previously approved these changes Jan 31, 2020
@google-ml-butler google-ml-butler bot added the ready to pull PR ready for merge process label May 28, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 28, 2020
@gbaned gbaned removed the stat:awaiting response Status - Awaiting response from author label May 29, 2020
@rthadur
Copy link
Contributor

rthadur commented Jun 1, 2020

@rushabh-v can you please check below errors

File "/testing/parameterized.py", line 284, in bound_param_test return test_method(self, *testcase_params) File "/tensorflow/python/keras/keras_parameterized.py", line 388, in decorated _v1_session_test(f, self, config, *args, **kwargs) File "/tensorflow/python/keras/keras_parameterized.py", line 405, in _v1_session_test f(test_or_class, *args, **kwargs) File "/tensorflow/python/keras/layers/preprocessing/text_vectorization_test.py", line 1214, in test_split_equals_zero_on_adapt predictions = model.predict(predict_data) File "/tensorflow/python/keras/engine/training_v1.py", line 988, in predict use_multiprocessing=use_multiprocessing) File "/tensorflow/python/keras/engine/training_arrays.py", line 707, in predict x, check_steps=True, steps_name='steps', steps=steps) File "/tensorflow/python/keras/engine/training_v1.py", line 2330, in _standardize_user_data batch_size=batch_size) File "/tensorflow/python/keras/engine/training_v1.py", line 2357, in _standardize_tensors exception_prefix='input') File "/tensorflow/python/keras/engine/training_utils.py", line 527, in standardize_input_data standardize_single_array(x, shape) for (x, shape) in zip(data, shapes) File "/tensorflow/python/keras/engine/training_utils.py", line 527, in <listcomp> standardize_single_array(x, shape) for (x, shape) in zip(data, shapes) File "/tensorflow/python/keras/engine/training_utils.py", line 452, in standardize_single_array if (x.shape is not None and len(x.shape) == 1 and AttributeError: 'str' object has no attribute 'shape'

@rushabh-v
Copy link
Contributor Author

hey @rthadur !

I apologize to you and @mihaimaruseac for not replying to your comments. Actually I mistakenly unsubscribed from the notification of this PR. And just saw those comments today.

But I see that the errors are solved with the last commit of @mihaimaruseac

@rthadur rthadur added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Jun 1, 2020
@mihaimaruseac mihaimaruseac added the ready to pull PR ready for merge process label Jul 7, 2020
@rthadur rthadur added the kokoro:force-run Tests on submitted change label Jul 7, 2020
@kokoro-team kokoro-team removed kokoro:force-run Tests on submitted change labels Jul 7, 2020
@rthadur
Copy link
Contributor

rthadur commented Jul 7, 2020

@rushabh-v we still see same error , can you please check again.

@mihaimaruseac
Copy link
Collaborator

It seems it's an internal error only, so probably will need manual import. Don't know if I can do this this week, but I'll add it on my plate.

@rushabh-v
Copy link
Contributor Author

Yes, the traceback says: AssertionError: Exception of type <class 'tensorflow.python.framework.errors_impl.UnimplementedError'>: Cast int32 to string is not supported [Op:Cast]

@gbaned gbaned added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Jul 13, 2020
@gbaned gbaned removed the ready to pull PR ready for merge process label Jul 21, 2020
@gbaned
Copy link
Contributor

gbaned commented Jul 24, 2020

@mihaimaruseac Any update on this PR? Please. Thanks!

@gbaned
Copy link
Contributor

gbaned commented Nov 5, 2020

@mihaimaruseac Any update on this PR? Please. Thanks!

@deeb02 deeb02 requested review from mattdangerw and removed request for tanzhenyu March 9, 2021 05:19
@gbaned
Copy link
Contributor

gbaned commented Mar 26, 2021

@mihaimaruseac Any update on this PR? Please. Thanks!

@mattdangerw
Copy link
Member

This looks pretty stale--this fix would no longer apply at head. And the underlying issue this fixes looks already closed.

I think we can probably just close this @gbaned

@gbaned
Copy link
Contributor

gbaned commented Mar 30, 2021

@mattdangerw Thanks for the confirmation. Closing this PR. Thanks!

@gbaned gbaned closed this Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:keras Keras related issues prtype:bugfix PR to fix a bug size:XS CL Change Size: Extra Small stat:awaiting tensorflower Status - Awaiting response from tensorflower
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't set None on TextVectorization layer's split parameter.
10 participants