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

Clean up examples/ directory #637

Merged
merged 13 commits into from
Jan 11, 2023

Conversation

ADITYADAS1999
Copy link
Contributor

Clean up examples/ directory #573

@mattdangerw
Copy link
Member

mattdangerw commented Jan 6, 2023

It would be nice to keep some sort of minimal content in the README...

# KerasNLP Examples

The `examples/` directly contains scripts built on top of the library that do not fit well into
the colab format used on [keras.io](https://keras.io/examples/). This includes recipes for
pre-training models and evaluating models on benchmarks such as GLUE.

We should do some light renaming of the content in this directory and fix all import after renaming.

  • Rename /examples/bert/ -> /examples/bert_pretraining/
  • Delete the file bert_finetune_glue.py (we now have better code in the glue_finetuning directly).
  • Rename the file bert_train.py -> bert_pretrain.py.
  • Rename the file bert_preprocess.py -> bert_create_pretraining_data.py.

@chenmoneygithub do those renames look ok to you as well?

@ADITYADAS1999
Copy link
Contributor Author

It would be nice to keep some sort of minimal content in the README...

# KerasNLP Examples

The `examples/` directly contains scripts built on top of the library that do not fit well into
the colab format used on [keras.io](https://keras.io/examples/). This includes recipes for
pre-training models and evaluating models on benchmarks such as GLUE.

Thanks mattdangerw, I add this to readme 👍🏻

@ADITYADAS1999
Copy link
Contributor Author

ADITYADAS1999 commented Jan 7, 2023

Hey @chenmoneygithub can I make this changes, If those renames look ok ?

  • Rename /examples/bert/ -> /examples/bert_pretraining/
  • Delete the file bert_finetune_glue.py (we now have better code in the glue_finetuning directly).
  • Rename the file bert_train.py -> bert_pretrain.py.
  • Rename the file bert_preprocess.py -> bert_create_pretraining_data.py.

@mattdangerw
Copy link
Member

@ADITYADAS1999 thanks! This looks good!

We will also need to update the README here -> https://github.com/keras-team/keras-nlp/blob/master/examples/bert/README.md to reflect the script renames. Should be a simpler find and replace.

Also can we delete this section entirely? https://github.com/keras-team/keras-nlp/blob/master/examples/bert/README.md#evaluating-bert-with-glue. We are removing the script on this PR.

@ADITYADAS1999
Copy link
Contributor Author

We will also need to update the README here -> https://github.com/keras-team/keras-nlp/blob/master/examples/bert/README.md to reflect the script renames. Should be a simpler find and replace.

what kind of changes should I change here in the docs? can you suggest little bit.

@ADITYADAS1999
Copy link
Contributor Author

Also can we delete this section entirely? https://github.com/keras-team/keras-nlp/blob/master/examples/bert/README.md#evaluating-bert-with-glue. We are removing the script on this PR.

In my opinion we have to remove this section, as we remove the bert_finetune_glue.py file. kept this extra section generate confusion for new maintainers and contributors in future.

@mattdangerw
Copy link
Member

@ADITYADAS1999

can you suggest little bit.

Sure!

image

@ADITYADAS1999
Copy link
Contributor Author

@ADITYADAS1999

can you suggest little bit.

Sure!

image

thanks mattdangerw

Copy link
Member

@mattdangerw mattdangerw left a comment

Choose a reason for hiding this comment

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

Thank you!

@mattdangerw mattdangerw merged commit 106ff77 into keras-team:master Jan 11, 2023
@ADITYADAS1999
Copy link
Contributor Author

Thank you!

Thanks mattdangerw for reviewing 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants