Analysis attributes are None when they shouldn't be

[thclark]

I'm submitting a ...

• support request
• bug report
• feature request

Please fill out the relevant sections below.

---

Bug report

What is the current behavior?

analysis.input_dir is None
>>> True
analysis.data_dir is None
>>> True
analysis.output_dir is None
>>> True
output_manifest is None
>>> True

What is the expected behavior?

• Analysis *_dir attributes should be either not available or should be not None (i.e. correct)
• Where a twine file has an output_manifest strand, the output manifest should be pre-created.
• There should be an example of their use in either the documentation of the demo apps

Other information

@time-trader please could you attach the twine file for this case that we discussed this afternoon?

[cortadocodes]

@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.

[cortadocodes]

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

[thclark]

@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)

[cortadocodes]

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

[cortadocodes]

@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]

@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.

[time-trader]

@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 ":"

[thclark]

@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.