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

Added Example Notebooks #1195

Closed
wants to merge 13 commits into from
Closed

Added Example Notebooks #1195

wants to merge 13 commits into from

Conversation

SauravMaheshkar
Copy link
Contributor

This is in reference to the following gitter discussions:

  1. trax.data Explained
  2. Deep N-Gram

@google-cla google-cla bot added the cla: yes label Nov 5, 2020
@afrozenator afrozenator added ready to pull Added when the PR is ready to be merged. and removed ready to pull Added when the PR is ready to be merged. labels Nov 16, 2020
@afrozenator
Copy link
Contributor

Hi @SauravMaheshkar - One quick request, can you add the following preamble (as a separate code block) to the notebook?

Thanks

#@title
# Copyright 2020 Google LLC.

# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at

# https://www.apache.org/licenses/LICENSE-2.0

# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

@SauravMaheshkar
Copy link
Contributor Author

Done @afrozenator

@afrozenator afrozenator added the ready to pull Added when the PR is ready to be merged. label Nov 19, 2020
@afrozenator
Copy link
Contributor

Another request, can you change the README.md to point the earlier added ipynb's locations to the TRAX locations?

Thanks a lot for these notebooks!

@SauravMaheshkar
Copy link
Contributor Author

I've added them to the README.md of this branch only. The links are the would be links of the TRAX repository (trax/blob/master/examples/trax-data-Explained.ipynb, ....)

@afrozenator
Copy link
Contributor

One more tiny request - Can you rebase to head?

I'm having difficulty pulling this PR in because when you created this the examples/ folder didn't exist internally -- I guess this will get solved by being on master's head.

I will investigate if there is any other way to pull this in.

@SauravMaheshkar
Copy link
Contributor Author

@afrozenator I added everything to my master branch. (deconvolution.py, README changes, examples/ folder). Should I open another PR from my master and close this PR?

@afrozenator
Copy link
Contributor

Sorry, GitHub isn't my strongest suit -- but it basically seems like on trax/master we have a README.md in examples/ that was deleted by rebasing (because now Github is saving this branch has conflicts that need resolving).

You maybe right - let's try it in a new PRs (on top of current TRAX head) just adding the notebooks in one PR and the deconv in another

Thanks - I'll let you close this.

@afrozenator
Copy link
Contributor

afrozenator commented Nov 19, 2020

(Or maybe just put everything into a single PR, less work for everyone)

Sorry for the back and forth, appreciate your patience!

@SauravMaheshkar
Copy link
Contributor Author

Done : #1232

@afrozenator
Copy link
Contributor

Closing this, since we're iterating on #1232

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull Added when the PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants