-
Notifications
You must be signed in to change notification settings - Fork 243
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
Remove file level docstrings #1222
Conversation
Pet peeve. These are inconsistent, usually just a rephrase of the file itself, and keras-core isn't doing them. Who doesn't like less code? We can keep out class/function docstrings as the place to focus on.
@@ -11,6 +11,7 @@ | |||
# 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. | |||
|
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.
Are all these new blank lines intentional? Version change in linter?
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.
Ooh this must be some find-replace shenanigans 😛
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.
Either way looks like it makes these files consistent with the rest so LGTM
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.
Yeah, we were not consistent, nor did our lint check. This was just a little script I wrote. No strong opinion on newline after copyright block or not.
@@ -11,6 +11,7 @@ | |||
# 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. | |||
|
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.
Either way looks like it makes these files consistent with the rest so LGTM
Pet peeve. These are inconsistent, usually just a rephrase of the file itself, and keras-core isn't doing them.
Let's ditch filename docstring (unless they actually say something interesting). We can keep out class/function docstrings as the place to focus on.