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

Update to commentary status #179

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

saengel
Copy link
Contributor

@saengel saengel commented Feb 20, 2024

This small script updates the status of each of the provided indices to commentary, and assigns a collective title to the indices which are each volumes of the same commentary on different books of Torah.

@saengel saengel self-assigned this Feb 20, 2024
Copy link
Contributor

@stevekaplan123 stevekaplan123 left a comment

Choose a reason for hiding this comment

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

You could also set the base_text_titles property on each Index, which would be an array of the books this comments on, so for the first one the value would be ["Genesis"]. That being said, if there isn't a base_text_mapping, there's probably not too much benefit to setting the base_text_titles. See line 209 in model/text.py.

@saengel
Copy link
Contributor Author

saengel commented Feb 20, 2024

You could also set the base_text_titles property on each Index, which would be an array of the books this comments on, so for the first one the value would be ["Genesis"]. That being said, if there isn't a base_text_mapping, there's probably not too much benefit to setting the base_text_titles. See line 209 in model/text.py.

Thanks @stevekaplan123 - when I checked about this, I was specifically told that we want to leave base_text_titles out of this for now, since we want this text to serve as a quoting commentary. Is that best practice? Or should I update? Let me know what you think, and thanks.

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