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

Refine readme.md #1292

Merged
merged 12 commits into from
Jan 28, 2023
Merged

Refine readme.md #1292

merged 12 commits into from
Jan 28, 2023

Conversation

LukeWood
Copy link
Contributor

@LukeWood LukeWood commented Jan 19, 2023

Currently we're really highlighting a few use cases in the 3P entrypoint. Our APIs are in no way restricted to these use cases (even if they are internally) and we should communicate that to 3p users.

The goal of the package is to solve all OD, Segmentation, and classification tasks; so lets make sure we communicate that clearly to users.


Note: leaving this PR open to discuss

README.md Outdated Show resolved Hide resolved
@LukeWood
Copy link
Contributor Author

@fchollet @jbischof @ianstenbit I updated the README a bit to try to be a bit more "community" oriented - please let me know what you think! Some copyedits too.

I consider our README to be one of the most important files in the repo - so please feel free to nitpick!

@LukeWood LukeWood requested review from bhack, chenmoneygithub, mattdangerw and qlzh727 and removed request for bhack January 26, 2023 19:05
@LukeWood
Copy link
Contributor Author

(added a few others as README is pretty important)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
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.

Thanks! Just left some quick comments, feel free to just cherry-pick whatever is useful!

README.md Outdated Show resolved Hide resolved
README.md Outdated

# Mission

KerasCV is a layered repository consisting of core components and modeling components.
KerasCV is a computer vision library of modular computer vision oriented Keras components.
Copy link
Member

Choose a reason for hiding this comment

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

It might be cool if we could align the first sentences of KerasCV's and KerasNLP's mission. For KerasNLP we have -> "KerasNLP is a natural language processing library that supports users through their entire development cycle." Def open to changing that if we find some language we think will be exciting for readers and applicable for both projects.

cc @jbischof

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to feedback. I think the CV selling point is a bit different in that its based on modularity - but open to feedback.

Copy link
Member

Choose a reason for hiding this comment

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

Modular only makes our second sentence currently :P, but I would be totally down to bump it up. Totally fine to leave both as is if this is feeling too tricky, was just thinking it might be a good way to show some alignment between the packages.

README.md Outdated Show resolved Hide resolved
@LukeWood
Copy link
Contributor Author

/gcbrun

README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jbischof jbischof left a comment

Choose a reason for hiding this comment

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

Seems very reasonable. Thanks @LukeWood!

One popular add for a README is a "quickstart" code snippet to show the "headline" CUJ and give the users a taste for the API (example). WDYT?

@bhack
Copy link
Contributor

bhack commented Jan 28, 2023

@jbischof If you are interested, some time ago, I've tried to propose some template sections to lower the cognitive overhead when navigating across ecosystem repos Readme files:

tensorflow/community#376

We have also the contributing markdown but almost without review:
tensorflow/community#384

@ianstenbit
Copy link
Contributor

Seems very reasonable. Thanks @LukeWood!

One popular add for a README is a "quickstart" code snippet to show the "headline" CUJ and give the users a taste for the API (example). WDYT?

I'm personally very much in favor of this idea (probably as a separate PR)

Copy link
Contributor

@ianstenbit ianstenbit left a comment

Choose a reason for hiding this comment

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

thanks Luke!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@jbischof
Copy link
Contributor

@jbischof If you are interested, some time ago, I've tried to propose some template sections to lower the cognitive overhead when navigating across ecosystem repos Readme files:

Thanks @bhack for this and your many other contributions! I don't want to force a big rewrite on @LukeWood at this time but would be happy if he wanted to follow the templates as well

@bhack
Copy link
Contributor

bhack commented Jan 28, 2023

Thanks @bhack for this and your many other contributions! I don't want to force a big rewrite on @LukeWood at this time but would be happy if he wanted to follow the templates as well

You are right but the full story begin one year ago. 😉

@LukeWood
Copy link
Contributor Author

Seems very reasonable. Thanks @LukeWood!

One popular add for a README is a "quickstart" code snippet to show the "headline" CUJ and give the users a taste for the API (example). WDYT?

Great idea - I even have one written: https://raw.githubusercontent.com/keras-team/keras-io/master/templates/keras_cv/index.md

added

@LukeWood
Copy link
Contributor Author

/gcbrun

@LukeWood
Copy link
Contributor Author

Thank you everyone for your reviews and comments!

@LukeWood LukeWood merged commit 76d3e4f into master Jan 28, 2023
@ianstenbit ianstenbit deleted the LukeWood-patch-1 branch July 1, 2023 00:45
ghost pushed a commit to y-vectorfield/keras-cv that referenced this pull request Nov 16, 2023
* Fix inference for jittered resize

* Fix jittered resize

* Inference test

* Inference test

* README update

* README update

* README update

* Ian comments

* Matt comment

* Matt comment

* Fix haifeng comment

* Fix Jonathan comment
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.

6 participants