Skip to content

Conversation

@saengel
Copy link
Contributor

@saengel saengel commented Jan 29, 2025

Script to rename the Bavli Chapters (English) with their transliterated names, as well as one correction for the Hebrew name of Bava Kamma 2.

@saengel saengel requested a review from yitzhakc January 29, 2025 13:10

django.setup()

from sefaria.model import *

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

Copy link
Contributor Author

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 *

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

Copy link
Contributor Author

@saengel saengel Feb 20, 2025

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

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