-
Notifications
You must be signed in to change notification settings - Fork 143
EncodeBase64 and DecodeBase64 ops #376
base: master
Are you sure you want to change the base?
Conversation
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 (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! approved with couple minor comments.
@@ -0,0 +1,64 @@ | |||
/** | |||
* @license | |||
* Copyright 2018 Google Inc. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update to 2019 for all files, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {Node} from '../types'; | ||
|
||
import {executeOp} from './string_executor'; | ||
// tslint:disable-next-line:max-line-length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed (c40b65c)
node.op = 'EncodeBase64'; | ||
node.attrParams.pad = createBoolAttr(true); | ||
executeOp(node, {input1}, context); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the empty line here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done (7663a37)
- per review comment
- per review comment
- per review comment
- per review request
@pyu10055 thank you, i have made the updates as requested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change depends on a PR with tfjs-core, will need to wait for tfjs-core merging that first, and publishing a new version.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
the TF implementation of the pix2pix model which fails conversion because of
the Open NSFW model also fails conversion with some of the same ops (tensorflow/tfjs#433).
i would like to take the opportunity to implement some of the ops in TensorFlow.js. starting with this pull request for
DecodeBase64
andEncodeBase64
.along with this
tfjs-converter
PR, there is a corresponding PR intfjs-core
(tensorflow/tfjs-core#1779)This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)