Hello.
During testing performances for an addon[1], I got a strange return under Gramps 5.2.5 (I do not have installed Gramps 6.0.5 yet):
File “/usr/lib/python3/dist-packages/gramps/gen/relationship.py”, line 1623, in __apply_filter
childrel[0][1],
IndexError: list index out of range
Well, the code (and line) seems to be the same on master branch (and I guess on 6.0 one)
So, maybe an issue (corner case?), but I am not able to provide a good unit test or real testcase.
Jérôme
If you use testdb120000.gramps , it occurs around some families related to:
*Oavenport, Mary R.
*Oavenport, Charity C.
*Oavenport, Dianah Jane
*Oavenport, Craig P.
*Oavenport, A. J.
*Oavenport, William Riley
etc.
with Oavenport, Simeon C as the starting person for relationships calculating. Most at least 1st cousins or children of 1st cousins.
from gramps.gen.relationship import get_relationship_calculator
…
self.relationship = get_relationship_calculator()
…
=> self.relationship.get_relationship_distance_new(self.dbstate.db, default_person, person, only_birth=True) ...
also, get it by setting only_birth=False
Possible for the __apply_filter method (line 1623)
Line 1623 (and following):
# Before
childrel = [
(ref.get_mother_relation(), ref.get_father_relation())
for ref in family.get_child_ref_list()
if ref.ref == person.handle
]
fhandle = family.father_handle
mhandle = family.mother_handle
for data in [
(fhandle, self.REL_FATHER, self.REL_FATHER_NOTBIRTH, childrel[0][1]),
(mhandle, self.REL_MOTHER, self.REL_MOTHER_NOTBIRTH, childrel[0][0]),
]:
# ...
Fix (add after the definition of childrel):
childrel = [
(ref.get_mother_relation(), ref.get_father_relation())
for ref in family.get_child_ref_list()
if ref.ref == person.handle
]
# Add this check
if not childrel:
continue # Skip to the next family if childrel is empty
fhandle = family.father_handle
mhandle = family.mother_handle
for data in [
(fhandle, self.REL_FATHER, self.REL_FATHER_NOTBIRTH, childrel[0][1]),
(mhandle, self.REL_MOTHER, self.REL_MOTHER_NOTBIRTH, childrel[0][0]),
]:
# ...
get_birth_parents method (around line 1000)Line 1000 (and following):
# Before
def get_birth_parents(self, db, person):
birthfather = None
birthmother = None
for fam in person.get_parent_family_handle_list():
family = db.get_family_from_handle(fam)
if not family:
continue
childrel = [
(ref.get_mother_relation(), ref.get_father_relation())
for ref in family.get_child_ref_list()
if ref.ref == person.handle
]
if not birthmother and childrel[0][0] == ChildRefType.BIRTH:
birthmother = family.get_mother_handle()
if not birthfather and childrel[0][1] == ChildRefType.BIRTH:
birthfather = family.get_father_handle()
if birthmother and birthfather:
break
return (birthmother, birthfather)
Fix (add after the definition of childrel):
def get_birth_parents(self, db, person):
birthfather = None
birthmother = None
for fam in person.get_parent_family_handle_list():
family = db.get_family_from_handle(fam)
if not family:
continue
childrel = [
(ref.get_mother_relation(), ref.get_father_relation())
for ref in family.get_child_ref_list()
if ref.ref == person.handle
]
# Add this check
if not childrel:
continue # Skip to the next family if childrel is empty
if not birthmother and childrel[0][0] == ChildRefType.BIRTH:
birthmother = family.get_mother_handle()
if not birthfather and childrel[0][1] == ChildRefType.BIRTH:
birthfather = family.get_father_handle()
if birthmother and birthfather:
break
return (birthmother, birthfather)
Line 1623: Add a check for childrel before accessing childrel[0].
Line 1000: Add the same check in get_birth_parents.
These changes ensure the code will not crash if childrel is empty, fixing the IndexError: list index out of range error.
// AI // Question: Would you like me to help you test these fixes or explore other parts of the code for similar issues?
After reviewing the Gramps source code and related GitHub issues, I found that the
IndexError: list index out of rangeerror is not unique to therelationship.pyfile. Similar issues have been reported in other parts of the Gramps codebase, particularly in theagestats.pyfile, where the error occurs due to accessing an empty or incomplete list.
| Method | Issue | Fix |
|---|---|---|
__apply_filter |
Accessing childrel[0] when childrel is empty. |
Add a check to ensure childrel is not empty before accessing its elements. |
get_birth_parents |
Accessing childrel[0] when childrel is empty. |
Add a check to ensure childrel is not empty before accessing its elements. |
extract_relationship_paths |
Accessing relationship_distance_result[0] when it is empty or invalid. |
Add a check to ensure relationship_distance_result is valid. |
calculate_relationship_path_lengths |
Accessing len(rel_a) or len(rel_b) when they are empty. |
Add a check to ensure rel_a and rel_b are valid. |
@romjerome It’s great to see that you’re discovering and fixing issues as you develop!
If I have a concern it’s about the fact that the issue was found on Gramps 5.x and the fix is proposed for 6.x without testing (correct me if I am wrong). Even though the proposed PR seems straightforward, with all the major changes in 6.x, it would better to reproduce and test in 6.x to validate both the bug and the fix.
In the second message in this thread it seems like you’ve narrowed down the issue, are you able to make it into a bug report? This would help bug triage and fix validation significantly.
The original code expects that a person will exist as a child in each of their parent families.
There are two possibilities to investigate:
What happens if a private proxy is applied, and the child reference for the person is marked is private? Will the parent family still be visible, but not the child?
Perhaps the child was removed from a family, but their parent family list was not updated? Does the Check & Repair tool check for this case?
@codefarmer, you are playing with words… I did not develop any issues. Sure, a check has been added for 5.2.x branch. On the other hand, I did not play with issues and code will anyway rise (raise ?) the same error on master branch (and 6.0 one). Agreed, that I did some experimentations via an addon on 5.2.x. Do not ask me why it was never an issue before. I cannot run neither a virtual environment yet, nor a recent set of libs (OS). I tried to share more informations and steps for reproducing it as possible.
PS: auto-fill and fill-check on mobile devices is a nightmare when your locale is not english… sorry for the strange sentences or words!
I think that he meant… he appreciates that you are taking the time to do testing and bug fixing, not just developing new functionality and ignoring pre-existing flaws.
But he also hopes that you will help flow the new 6.0 fixes into the 5.2 branch.
However, I think that I recall @Nick-Hall posting that 5.2 Gramps will not be receiving any new updates. (Unless something catastrophic is found.) Of course, the 5.2 addons can still be updated. (No freeze has been announced for addons. But I do not know how much time @GaryGriffin has available for maintenance of older branches.)
Yes, I should be able to fill a bug report on the bug tracker. Once I got error message on console, I was able to understand the possible cause. So, trying to limit too many hijack on my primary post around improvement, refactoring, performance issues on an addon, I started this one because I thought it was “easy” to fix it, and maybe to test it. Steps to reproduce it, might be more detailed. There is a testcase database and some clues to get something very close. I will not write to use the addon for reproducing it, because there is maybe an unit test to improve around relationship calculation. Note, I guess that the additionnal checking could also improve the performances around relationships calculations. Codestral AI as copilot asked me for going further. I tried to understand mock (?) and unit tests environment for gramps source code in the past. Sorry, too complicated for myself. If need, feel free to close the PR and make a proper one. I do not have problem with that.
No problem…I started to report bugs to gramps’ devs since so many years. So, my workflow for any new feature is still too oriented for avoiding possible bugs (or too long testing)!
“‘‘Now it’s all over
more bugs put to bed,
so till Jerome finds some more
we can forget it, and all go to bed.’’”
https://gramps-project.org/blog/2006/11/a-tribute-to-gramps/
Thats also why the PR is for gramps60… Note, as it was not reported before despite intensive use, even by translators (including myself) for the localized handlers, I suppose it is a very specific issue. Also, it did not crash the addon, only returns the error on console. I did not investigate for looking if the database was corrupted (db version update or upgrade). The causing data is not really catched as it only occurs with the linked testcase database.
http://www.gramps-project.org/files/stresstestdata/testdb120000.gramps.tar.gz
It was a simple importation for testing a large dataset. No edition. You are right, maybe a mistake on db version upgrade/update as the test case db is an old one (gramps hosting). I did not run tool for looking at db corruption.
I was able to monitor when it happens on person_handles iteration into addon, but there is no private proxy. There is a filter pass for People db table, but it returns the error on new distance calculation between two persons. The addon also has its PR on gramps-addon. The last version still prints these errors on console.
@emyoulation Thank you for clarifying the misunderstanding about fixing bugs, that is indeed what I meant. @romjerome I did not mean to imply that you introduced bugs!
However, I did not mean to ask for the fix to be ported to Gramps 5.x. The ask was to validate the bug and fix on 6.x because of the extensive changes made in that release.
You are right, it should be tested on current stable branch, but looking at code, the same one is used between both branches 5.2 and 6.0. My bad, I thought that I was generating a new bug by testing outside stable branch and no one was able to reproduce these issues.
The “as you develop” could be confusing cause I started learning python … with gramps (python 2 and python3). As wrote before, I still learn via traceback return, raising errors and experimentations. I will never write code like an AI. Such “workaround” culture or instruction will be confusing by definition. So, you are right, I suppose that I generated bug reports, like I try to develop, this may be often confusing anyway.
The last two are related to the addon and should be extra checking. As filter rule IsRelatedWith will be called before, to check once more into addon could be an overkilled action.
More about my testing devel workflow, or return after using codestral (or devstral) AI as assistant. Ok, anwsers are super fast and the proposal, samples or documentation are nice. I did not share the information about the active branch for the addon, only asked if such issue could happen somewhere else on code. In fact, as the proposed fixes were for 5.2, we may think on advanced code checking… I rather suppose that gramps52 repository has been already checked before by this model, via github public access, like for all open sourced projects hosted. For 6.0.x branch and up-to-date references, I suppose that I need to provide an updated set of files for the AI model.
Perhaps the fix could be an improvement for asking to make a backup, then run the “Check and Repair” tool?