-
Notifications
You must be signed in to change notification settings - Fork 63
Script for Bavli Chapter Renames #183
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
base: master
Are you sure you want to change the base?
Script for Bavli Chapter Renames #183
Conversation
|
|
||
| django.setup() | ||
|
|
||
| from sefaria.model import * |
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.
Did you purposely put this import below the django.setup()? PEP8 recommends having all the imports at the top
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.
Yes, if I recall correctly, django.setup() is necessary before importing the model, although it lends itself to a clunky import section at the top of the file.
|
|
||
| django.setup() | ||
|
|
||
| from sefaria.model import * |
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.
PEP8 recommends not to import everything from a package (... import *) and only import the things you need, even when it's a long list - due to potential namespace pollution and reduced code readability
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.
Correct, except for sefaria.model I was told this is best practice.
See here
Script to rename the Bavli Chapters (English) with their transliterated names, as well as one correction for the Hebrew name of Bava Kamma 2.