Commit debad4aa authored by Zebedee Nicholls's avatar Zebedee Nicholls
Browse files

Merge branch 'fix-up-code-coverage' into 'main'

Increase code coverage

See merge request !104
parents eeb23fb5 22484d20
Loading
Loading
Loading
Loading
Loading
+24 −0
Original line number Diff line number Diff line
Increased test coverage.

As part of fixing the test coverage, the following changes were made
These were unreleased at the time of merging, hence are marked as trivial.

Renamed {py:func}`fgen.jinja_environment.ensure_indent` to {py:func}`fgen.jinja_environment.indent_based_on_first_line`.
This also included fixing the behaviour of this function.
Previously there was a bug where the indent of the first line controlled the indenting of all further lines.
This is fixed now and clarified by the change in function name.

Renamed {py:func}`fgen.wrapper_building.file_should_be_written` to {py:func}`fgen.wrapper_building.write_file_with_checks`.
This was done to simplify the logic of the control flow.
Previously, the control flow was a blend of error raising and return flags, which was confusing.
Now, it is reverted to being handled in one place, which makes the logic much easier to follow.

Changed the following methods to properties:

- {py:attr}`fgen.data_models.fortran_derived_type.FortranDerivedType.exposed_attributes`
- {py:attr}`fgen.data_models.fortran_derived_type.FortranDerivedType.units`
- {py:attr}`fgen.data_models.fortran_derived_type.FortranDerivedType.units_multi_return`
- {py:attr}`fgen.data_models.method.Method.units`
- {py:attr}`fgen.data_models.fortran_parsing.FortranDataType.is_derived_type`
- {py:attr}`fgen.data_models.fortran_parsing.FortranDataType.is_pointer`
- {py:attr}`fgen.data_models.fortran_parsing.FortranDataType.is_deferred_array`
+55 −27
Original line number Diff line number Diff line
@@ -30,21 +30,58 @@ class FortranDerivedType:
    methods: dict[str, Method]
    """The derived type's methods"""

    # We may need the equivalent of this for methods.
    # I haven't thought through whether it makes sense to have a method
    # that requires a Fortran unit holder without any attribute
    # that has that requirement.
    @attributes.validator
    def _check_units_handling(self, attribute: Any, value: dict[str, Value]) -> None:
        requires_fortran_units_holder = self.fortran_units_holder_reliant_attributes
        if requires_fortran_units_holder:
        attributes_requiring_fortran_units_holder = (
            self.fortran_units_holder_reliant_attributes
        )
        if attributes_requiring_fortran_units_holder:
            if self.fortran_units_holder is None:
                msg = (
                    "There are attributes with dynamic units, "
                    "but no attribute is specified as a ``fortran_units_holder``. "
                    "Attributes with dynamic units "
                    "that require a Fortran units holder: "
                    f"{requires_fortran_units_holder}."
                    f"{attributes_requiring_fortran_units_holder}. "
                    f"{self=} {attribute=} {value=}"
                )
                raise ValueError(msg)

    @attributes.validator
    def _check_fortran_units_holder(
        self, attribute: Any, value: dict[str, Value]
    ) -> None:
        fortran_units_holders = [
            name
            for name, att in self.attributes.items()
            if att.definition.is_fortran_units_holder
        ]
        if not fortran_units_holders:
            # No Fortran units holders, fine here
            # (checked in _check_units_handling instead).
            return

        if len(fortran_units_holders) > 1:
            # Validation should prevent getting here, just in case
            msg = (
                "Cannot have more than one Fortran units holder. "
                "These attributes have `is_fortran_units_holder=True`: "
                f"{fortran_units_holders}"
            )
            raise ValueError(msg)

        fortran_units_holder = fortran_units_holders[0]
        if not self.attributes[fortran_units_holder].definition.expose_getter_to_python:
            msg = (
                "The Fortran units holder's getter must be exposed to Python. "
                f"This is not the case for `{fortran_units_holder}`"
            )
            raise ValueError(msg)

    @property
    def fortran_units_holder_reliant_attributes(self) -> tuple[str, ...]:
        """
@@ -87,31 +124,22 @@ class FortranDerivedType:
            for name, att in self.attributes.items()
            if att.definition.is_fortran_units_holder
        ]

        if not fortran_units_holders:
            return None

        if len(fortran_units_holders) == 1:
            fortran_units_holder = fortran_units_holders[0]
            if not self.attributes[
                fortran_units_holder
            ].definition.expose_getter_to_python:
                msg = (
                    "The Fortran units holder's getter must be exposed to Python. "
                    f"This is not the case for {fortran_units_holder}"
                )
                raise ValueError(msg)

            return fortran_units_holder

        if len(fortran_units_holders) > 1:  # pragma: no cover
            # Validation should prevent getting here, just in case
            msg = (
                "Validation should have prevented this. "
                "Cannot have more than one Fortran units holder. "
                "These attributes have `is_fortran_units_holder=True`: "
                f"{fortran_units_holders}"
            )
        raise ValueError(msg)
            raise AssertionError(msg)

    # TODO: Make this a property
        return fortran_units_holders[0]

    @property
    def exposed_attributes(self) -> dict[str, Value]:
        """
        Get the attributes that are marked to be exposed to python
@@ -127,7 +155,7 @@ class FortranDerivedType:
            )
        )

    # TODO: Make this a property
    @property
    def units(self) -> dict[str, Union[str, tuple[str, ...]]]:
        """
        Get the unit for each declared value in the derived type
@@ -162,7 +190,7 @@ class FortranDerivedType:

        # Collect units from the methods
        for method_name, method in self.methods.items():
            method_units = method.units()
            method_units = method.units

            # Check that the method units are consistent
            # with any previously declared units
@@ -187,7 +215,7 @@ class FortranDerivedType:

        return {key: value[1] for key, value in unit_sources.items()}

    # TODO: Make this a property
    @property
    def units_multi_return(self) -> dict[str, tuple[str, ...]]:
        """
        Get units declared in this object, if the units are tuples
@@ -200,7 +228,7 @@ class FortranDerivedType:
            Dictionary containing value names' as keys and the associated units
            as values (all these values are plain strings).
        """
        return {k: v for k, v in self.units().items() if not isinstance(v, str)}
        return {k: v for k, v in self.units.items() if not isinstance(v, str)}

    def get_dynamic_unit_source(
        self, value: Union[MultiReturn, Value]
+1 −0
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ class Method:
    returns: Union[Value, MultiReturn]
    """Return value of the function"""

    @property
    def units(self) -> dict[str, Union[str, tuple[str, ...]]]:
        """
        Units used in the method
+1 −4
Original line number Diff line number Diff line
@@ -4,7 +4,7 @@ Data model of a value, excluding units

from __future__ import annotations

from typing import Any, Optional
from typing import Any

from attrs import define, field, validators

@@ -72,9 +72,6 @@ class UnitlessValue:
    If ``True``, we expose a setter for this value to Python, otherwise we don't.
    """

    # TODO: try removing this and see what happens, it doesn't seem to be used
    truncated_name: Optional[str] = None

    @fortran_type.validator
    def _check_fortran_type(self, attribute: Any, value: str) -> None:
        try:
+44 −37
Original line number Diff line number Diff line
@@ -704,6 +704,47 @@ class FortranDataType:
        """
        return "real" in self.type_specification and "8" in self.type_specification

    @property
    def is_derived_type(self) -> bool:
        """
        Whether this instance represents a type which is a derived type

        Returns
        -------
            ``True`` if self represents a type which is a derived type
            ``False`` otherwise.
            True if a type is a derived type, False if it is an intrinsic type
        """
        return self.type_specification.startswith("type(")

    @property
    def is_pointer(self) -> bool:
        """
        Whether this instance represents a type which is a pointer

        Returns
        -------
            ``True`` if self represents a type which is a pointer
            ``False`` otherwise.
        """
        return "pointer" in self.attribute_specifications

    @property
    def is_deferred_array(self) -> bool:
        """
        Whether this instance represents a deferred size array

        Returns
        -------
            ``True`` if self represents a type which is a deferred size array
            ``False`` otherwise.
        """
        dim_attrs = self.dimension_attribute_specification

        if dim_attrs is None:
            return False
        return ":" in dim_attrs.dimensions

    @property
    def is_enum(self) -> bool:
        """
@@ -720,7 +761,6 @@ class FortranDataType:
            "integer" in self.type_specification and "kind(" in self.type_specification
        )

    # TODO: clean up old API and docstrings below here
    @property
    def equivalent_python_type(self) -> str:
        """
@@ -728,15 +768,12 @@ class FortranDataType:

        The "base" type of the equivalent Python type will be:

        * `str` for `character`-based types
        * `bool` for `logical`-based types
        * `pint.Quantity` for other intrinsic types such as `real` and `integer`
        * a custom type for (Fortran) derived types

        If the Fortran type has a "dimension" attribute specification,
        this modifies the "base" type to a `tuple`
        that respects the intended shape of the value.
        n-dimensional variables are supported.
        TODO: remove this behaviour, doesn't scale well to long arrays
        and doesn't help with deferred shape arrays.
        """
        base_type = self._base_python_type()
        dimension_attribute_specification = self.dimension_attribute_specification
@@ -765,7 +802,7 @@ class FortranDataType:
            that is equivalent to the base type
            (i.e. ignoring any array or other container) of ``self``
        """
        if self.is_derived_type():
        if self.is_derived_type:
            match = re.match(
                _DERIVED_TYPE_REGEX, self.type_specification, flags=re.IGNORECASE
            )
@@ -808,36 +845,6 @@ class FortranDataType:
                self.type_specification
            )

    def is_derived_type(self) -> bool:
        """
        Check if the fortran type is a derived type

        Returns
        -------
            True if a type is a derived type, False if it is an intrinsic type
        """
        return self.type_specification.startswith("type(")

    def is_pointer(self) -> bool:
        """
        Check if the fortran type is a pointer

        Returns
        -------
            True if a type is a pointer, False otherwise
        """
        return "pointer" in self.attribute_specifications

    def is_deferred_array(self) -> bool:
        """
        Check if the fortran type is an array with at least one deferred dimension
        """
        dim_attrs = self.dimension_attribute_specification

        if dim_attrs is None:
            return False
        return ":" in dim_attrs.dimensions

    @property
    def dimension_attribute_specification(
        self,
Loading