Analysis attributes are None when they shouldn't be about octue-sdk-python HOT 8 CLOSED

@time-trader I think one part of the problem is a typo in the output_manifest key - there shouldn't be a : at the end. This is what's in your twine file currently:

  "output_manifest:": [
    {
      "key": "open_foam_result",
      "purpose": "A dataset containing solution fields of an openfoam case."
    },
    {
      "key": "airfoil_cp_values",
      "purpose": "A file containing cp values"
    }
  ]

@cortadocodes Yes. That was a problem. I will go cry in the corner for not noticing that ":"

from octue-sdk-python.

@time-trader I think I've fixed this but please send me your Twine file and a minimal amount of code that reproduces the issue so I can make sure.

from octue-sdk-python.

@thclark I'm not sure the second and third tickboxes are related to this issue now?

from octue-sdk-python.

@thclark I'm not sure the second and third tickboxes are related to this issue now?

Can you make a test where twine file specifies an output manifest, then assert that the value of output_manifest is not none in run()?

On the documentation, this seems like a weird place to start so happy with skipping that. Although I have a strong feeling we should actually deprecate input_dir on the analysis object to dissuade people from using it. It's best to use the path property on the Dataset (which is a Pathable) in question (that way we can ultimately allow datasets to reside in different locations)

from octue-sdk-python.

Here is the twine that resulted in this bug: https://gitlab.com/windenergie-hsr/aerosense/digital-twin/fsi-twins/foam_2d_twine/-/blob/dev/twine.json

from octue-sdk-python.

@time-trader I think one part of the problem is a typo in the output_manifest key - there shouldn't be a : at the end. This is what's in your twine file currently:

  "output_manifest:": [
    {
      "key": "open_foam_result",
      "purpose": "A dataset containing solution fields of an openfoam case."
    },
    {
      "key": "airfoil_cp_values",
      "purpose": "A file containing cp values"
    }
  ]

from octue-sdk-python.

@thclark the other problem appears to be in the Runner._update_manifest_path method:

@staticmethod
def _update_manifest_path((manifest, pathname):
    if manifest is not None and hasattr(pathname, "endswith"):
        if pathname.endswith(".json"):
            manifest.path = os.path.split(pathname)[0]

        # Otherwise do nothing and rely on manifest having its path variable set already
        return manifest

This returns None if the pathname is None, erasing the manifest if it exists.

I might be lacking some context here, but it seems incorrect that a Manifest can have a path at the same time as the output_manifest_path argument of the Runner.run method is None.

from octue-sdk-python.

@time-trader please don't cry. If you promise not to cry we'll promise to move octue/twined#16 up the priority board to prevent exactly this scenario from happening.

from octue-sdk-python.