avendesora / pythonbible

A python library for validating, parsing, normalizing scripture references and retrieving scripture texts (for open source and public domain versions)
https://docs.python.bible
MIT License
60 stars 12 forks source link

BUG: get_verse_text(67001009) gives wrong value #118

Closed nathanjmcdougall closed 1 year ago

nathanjmcdougall commented 1 year ago

1 Esdras 1:9 has verse ID 67001009.

Calling get_verse_text(67001009) gives the entire text of the hebrew bible and new testament, instead of the verse in question. This appears to be true regardless of the value of the version argument.

It also appears to be true for subsequent verses, e.g. 67001010, etc.

I am using pythonbible==0.11.0.

avendesora commented 1 year ago

I agree that this is definitely a bug, and I appreciate you pointing it out.

The bug fix will probably be to return an empty value if the given verse ID is a valid verse ID but doesn't exist in the text of the given version.

The pythonbible library currently doesn't include a version that has the text from 1 Esdras (or other books in the Apocrypha). The KJV did originally include the Apocrypha, but the edition I had access to in a usable format did not (and most published editions from the last couple of centuries do not either). I will work on obtaining an edition that does but will fix this bug in the meantime.

Thanks again for letting me know.

nathanjmcdougall commented 1 year ago

Sounds good. Perhaps rather than an empty value, it would be better to raise NotImplementedError? And for version + verse combinations which simply do not exist, a custom MissingVerseError?

avendesora commented 1 year ago

Yes, I agree with you. Raising an error would be more correct, and I will implement it that way.

I had originally considered this but decided against it out of concern for situations where a single verse is omitted from a version and I don't want to raise an error in that case, since I eventually want to be able to return footnotes, translation notes, etc. that may be associated with that verse.

For example, Matthew 18:11 exists in the King James version but is omitted from the American Standard version with the following translation note: "Many authorities, some ancient, insert v. 11. For the son of man came to save that which was lost. See Luk 19:10." I eventually want to optionally return that translation note but, for now, just return an empty value.

After a bit of testing, it's not going to be an issue to support the use case I was concerned about while also raising an error for verses (and generally books and chapters) that just aren't a part of the given version at all, such as 1 Esdras 1:9 in the original example. For those cases, we will raise an error and leave it up to the caller of the function to determine what to do at that point.

avendesora commented 1 year ago

This should be fixed in the latest release v0.11.1.