Skip to content

Update to commentary status - #179

Open
saengel wants to merge 2 commits into
masterfrom
feature/sc-25101/assign-commentary-dependence-to-sacks-books
Open

Update to commentary status#179
saengel wants to merge 2 commits into
masterfrom
feature/sc-25101/assign-commentary-dependence-to-sacks-books

Conversation

@saengel

@saengel saengel commented Feb 20, 2024

Copy link
Copy Markdown
Contributor

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

@stevekaplan123 stevekaplan123 left a comment

Copy link
Copy Markdown
Member

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

saengel commented Feb 20, 2024

Copy link
Copy Markdown
Contributor Author

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