diff --git a/gedcom/element/element.py b/gedcom/element/element.py index 3428e3c..ada376f 100644 --- a/gedcom/element/element.py +++ b/gedcom/element/element.py @@ -63,6 +63,16 @@ class Element(object): """ def __init__(self, level, pointer, tag, value, crlf="\n", multi_line=True): + """Creates a new GEDCOM element. + :type level: int + :type pointer: str + :type tag: str + :type value: str + :param crlf: line ending appended when serialising the element + :type crlf: str + :param multi_line: when True, long values are split across CONC/CONT continuation lines + :type multi_line: bool + """ # basic element info self.__level = level self.__pointer = pointer @@ -131,7 +141,8 @@ def __available_characters(self): return 0 if element_characters > 255 else 255 - element_characters def __line_length(self, line): - """@TODO Write docs. + """Returns the number of characters from `line` that fit within the available space of a GEDCOM line (max 255 chars). + Trims trailing spaces to avoid splitting a word across lines. :type line: str :rtype: int """ @@ -147,7 +158,8 @@ def __line_length(self, line): return available_characters - spaces def __set_bounded_value(self, value): - """@TODO Write docs. + """Sets this element's value to as many characters of `value` as fit within GEDCOM line limits. + Returns the number of characters consumed. :type value: str :rtype: int """ @@ -156,7 +168,8 @@ def __set_bounded_value(self, value): return line_length def __add_bounded_child(self, tag, value): - """@TODO Write docs. + """Adds a child element with the given `tag` and as many characters of `value` as fit within GEDCOM line limits. + Returns the number of characters consumed. :type tag: str :type value: str :rtype: int @@ -165,8 +178,8 @@ def __add_bounded_child(self, tag, value): return child.__set_bounded_value(value) def __add_concatenation(self, string): - """@TODO Write docs. - :rtype: str + """Splits `string` into GEDCOM CONC child elements, each fitting within line length limits. + :type string: str """ index = 0 size = len(string) diff --git a/gedcom/element/family.py b/gedcom/element/family.py index c91b5a9..f5bd899 100644 --- a/gedcom/element/family.py +++ b/gedcom/element/family.py @@ -32,10 +32,15 @@ class NotAnActualFamilyError(Exception): + """Raised when an operation requires a FamilyElement but a different element type is provided.""" pass class FamilyElement(Element): + """Represents a GEDCOM family (FAM) record.""" def get_tag(self): + """Returns the GEDCOM tag for this element. + :rtype: str + """ return gedcom.tags.GEDCOM_TAG_FAMILY diff --git a/gedcom/element/file.py b/gedcom/element/file.py index d02e8eb..d932efc 100644 --- a/gedcom/element/file.py +++ b/gedcom/element/file.py @@ -32,10 +32,15 @@ class NotAnActualFileError(Exception): + """Raised when an operation requires a FileElement but a different element type is provided.""" pass class FileElement(Element): + """Represents a GEDCOM file (FILE) record.""" def get_tag(self): + """Returns the GEDCOM tag for this element. + :rtype: str + """ return gedcom.tags.GEDCOM_TAG_FILE diff --git a/gedcom/element/individual.py b/gedcom/element/individual.py index da61d08..18cbe90 100644 --- a/gedcom/element/individual.py +++ b/gedcom/element/individual.py @@ -34,12 +34,17 @@ class NotAnActualIndividualError(Exception): + """Raised when an operation requires an IndividualElement but a different element type is provided.""" pass class IndividualElement(Element): + """Represents a GEDCOM individual (INDI) record.""" def get_tag(self): + """Returns the GEDCOM tag for this element. + :rtype: str + """ return gedcom.tags.GEDCOM_TAG_INDIVIDUAL def is_deceased(self): @@ -119,6 +124,9 @@ def get_name(self): return given_name, surname def get_all_names(self): + """Returns a list of all name values for this individual. + :rtype: list of str + """ return [a.get_value() for a in self.get_child_elements() if a.get_tag() == gedcom.tags.GEDCOM_TAG_NAME] def surname_match(self, surname_to_match): @@ -182,7 +190,7 @@ def get_birth_data(self): return date, place, sources def get_birth_year(self): - """Returns the birth year of a person in integer format + """Returns the birth year of a person as an integer, or -1 if no birth date is set or the year cannot be parsed. :rtype: int """ date = "" @@ -222,7 +230,7 @@ def get_death_data(self): return date, place, sources def get_death_year(self): - """Returns the death year of a person in integer format + """Returns the death year of a person as an integer, or -1 if no death date is set or the year cannot be parsed. :rtype: int """ date = "" @@ -247,7 +255,7 @@ def get_burial(self): ::deprecated:: As of version 1.0.0 use `get_burial_data()` method instead :rtype: tuple """ - self.get_burial_data() + return self.get_burial_data() def get_burial_data(self): """Returns the burial data of a person formatted as a tuple: (`str` date, `str´ place, `list` sources) @@ -278,7 +286,7 @@ def get_census(self): ::deprecated:: As of version 1.0.0 use `get_census_data()` method instead :rtype: list of tuple """ - self.get_census_data() + return self.get_census_data() def get_census_data(self): """Returns a list of censuses of an individual formatted as tuples: (`str` date, `str´ place, `list` sources) diff --git a/gedcom/element/object.py b/gedcom/element/object.py index 0090e2f..0fcc117 100644 --- a/gedcom/element/object.py +++ b/gedcom/element/object.py @@ -32,10 +32,12 @@ class NotAnActualObjectError(Exception): + """Raised when an operation requires an ObjectElement but a different element type is provided.""" pass class ObjectElement(Element): + """Represents a GEDCOM multimedia object (OBJE) record.""" def is_object(self): """Checks if this element is an actual object diff --git a/gedcom/parser.py b/gedcom/parser.py index 74f40c7..aa44d64 100644 --- a/gedcom/parser.py +++ b/gedcom/parser.py @@ -48,7 +48,7 @@ class GedcomFormatViolationError(Exception): - pass + """Raised when a GEDCOM format violation is encountered during parsing.""" class Parser(object): @@ -280,7 +280,7 @@ def __build_list(self, element, element_list): def get_marriages(self, individual): """Returns a list of marriages of an individual formatted as a tuple (`str` date, `str` place) :type individual: IndividualElement - :rtype: tuple + :rtype: list of tuple """ marriages = [] if not isinstance(individual, IndividualElement): @@ -448,10 +448,13 @@ def get_parents(self, individual, parent_type="ALL"): return parents def find_path_to_ancestor(self, descendant, ancestor, path=None): - """Return path from descendant to ancestor - :rtype: object + """Return the path of IndividualElements from descendant to ancestor, or None if no path exists. + + :type descendant: IndividualElement + :type ancestor: IndividualElement + :rtype: list of IndividualElement or None """ - if not isinstance(descendant, IndividualElement) and isinstance(ancestor, IndividualElement): + if not isinstance(descendant, IndividualElement) or not isinstance(ancestor, IndividualElement): raise NotAnActualIndividualError( "Operation only valid for elements with %s tag." % gedcom.tags.GEDCOM_TAG_INDIVIDUAL ) diff --git a/tests/element/test_individual.py b/tests/element/test_individual.py index f5dfabd..94bceb2 100644 --- a/tests/element/test_individual.py +++ b/tests/element/test_individual.py @@ -19,3 +19,55 @@ def test_get_all_names(): all_names = element.get_all_names() assert len(all_names) == 2 + + +def test_deprecated_get_burial_returns_data(): + """Regression test: get_burial() is missing a return statement and returns None instead of the burial tuple.""" + element = IndividualElement(level=0, pointer="@I1@", tag="INDI", value="") + burial = element.new_child_element(tag=gedcom.tags.GEDCOM_TAG_BURIAL, value="") + burial.new_child_element(tag=gedcom.tags.GEDCOM_TAG_DATE, value="1 JAN 2000") + burial.new_child_element(tag=gedcom.tags.GEDCOM_TAG_PLACE, value="Szczecinek") + + result = element.get_burial() + + assert result is not None, "get_burial() returned None — missing return statement bug" + assert result == ("1 JAN 2000", "Szczecinek", []) + + +def test_deprecated_get_census_returns_data(): + """Regression test: get_census() is missing a return statement and returns None instead of the census list.""" + element = IndividualElement(level=0, pointer="@I1@", tag="INDI", value="") + census = element.new_child_element(tag=gedcom.tags.GEDCOM_TAG_CENSUS, value="") + census.new_child_element(tag=gedcom.tags.GEDCOM_TAG_DATE, value="1 JAN 1950") + census.new_child_element(tag=gedcom.tags.GEDCOM_TAG_PLACE, value="Szczebrzeszyn") + + result = element.get_census() + + assert result is not None, "get_census() returned None — missing return statement bug" + assert len(result) == 1 + assert result[0] == ("1 JAN 1950", "Szczebrzeszyn", []) + + +def test_get_burial_data_returns_data(): + element = IndividualElement(level=0, pointer="@I1@", tag="INDI", value="") + burial = element.new_child_element(tag=gedcom.tags.GEDCOM_TAG_BURIAL, value="") + burial.new_child_element(tag=gedcom.tags.GEDCOM_TAG_DATE, value="1 JAN 2000") + burial.new_child_element(tag=gedcom.tags.GEDCOM_TAG_PLACE, value="Szczecinek") + + result = element.get_burial_data() + + assert result is not None + assert result == ("1 JAN 2000", "Szczecinek", []) + + +def test_get_census_data_returns_data(): + element = IndividualElement(level=0, pointer="@I1@", tag="INDI", value="") + census = element.new_child_element(tag=gedcom.tags.GEDCOM_TAG_CENSUS, value="") + census.new_child_element(tag=gedcom.tags.GEDCOM_TAG_DATE, value="1 JAN 1950") + census.new_child_element(tag=gedcom.tags.GEDCOM_TAG_PLACE, value="Szczebrzeszyn") + + result = element.get_census_data() + + assert result is not None + assert len(result) == 1 + assert result[0] == ("1 JAN 1950", "Szczebrzeszyn", []) diff --git a/tests/test_parser.py b/tests/test_parser.py index 86df008..3d8be22 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1,4 +1,6 @@ -from gedcom.element.individual import IndividualElement +import pytest +from gedcom.element.element import Element +from gedcom.element.individual import IndividualElement, NotAnActualIndividualError from gedcom.element.root import RootElement from gedcom.parser import Parser @@ -101,3 +103,31 @@ def test_parse_from_string(): def test___parse_line(): # @TODO Add appropriate testing cases pass + + +def test_find_path_to_ancestor_raises_for_invalid_ancestor(): + """Regression test for the 'and' vs 'or' bug in find_path_to_ancestor. + + The guard condition uses 'and' instead of 'or', so passing a valid + descendant with an invalid ancestor silently skips the check and crashes + later with an AttributeError instead of NotAnActualIndividualError. + """ + parser = Parser() + individual = IndividualElement(0, '@I1@', 'INDI', '', '\n') + non_individual = Element(0, '', 'NOTE', 'some note', '\n') + + # valid descendant, invalid ancestor — the bug lets this slip through + with pytest.raises(NotAnActualIndividualError): + parser.find_path_to_ancestor(individual, non_individual) + + +def test_find_path_to_ancestor_raises_for_both_invalid(): + """Both arguments invalid — the buggy 'and' condition evaluates to False + and skips the guard entirely, crashing later instead of raising properly. + """ + parser = Parser() + non_individual_1 = Element(0, '', 'NOTE', 'some note', '\n') + non_individual_2 = Element(0, '', 'SOUR', 'some source', '\n') + + with pytest.raises(NotAnActualIndividualError): + parser.find_path_to_ancestor(non_individual_1, non_individual_2)