Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CircuitConfig.expanded_json does not expand relative paths #152

Open
joni-herttuainen opened this issue Jul 23, 2021 · 5 comments
Open

CircuitConfig.expanded_json does not expand relative paths #152

joni-herttuainen opened this issue Jul 23, 2021 · 5 comments
Assignees

Comments

@joni-herttuainen
Copy link

E.g., with a config file having nodes_file and edges_file defined as follows:

{
    "version": 2,
    "networks": {
        "nodes": [
            {
                "nodes_file": "./nodes.h5",
                ...
            }
        ],
        "edges": [
            {
                "edges_file": "edges.h5",
                ...
                }
            }
        ]
    }
}

I can access the data in the the files but when getting the expanded_json but I don't have the full paths as I would've expected:
'{"networks":{"edges":[{"edges_file":"edges.h5",...}],"nodes":[{"nodes_file":"./nodes.h5",...}]},"version":2}'

@NadirRoGue NadirRoGue self-assigned this Jul 26, 2021
@NadirRoGue
Copy link
Contributor

Hello,
The expanded JSON function replaces the manifest variables anywhere in the file where they are being used, like in this example:

{
  "target_simulator": "NEURON",
  "manifest": {
    "$BASE_DIR": "./",
    "$COMPONENT_DIR": "$BASE_DIR",
    "$NETWORK_DIR": "../"
  },
  "components": {
    "morphologies_dir": "$COMPONENT_DIR/morphologies",
    "biophysical_neuron_models_dir": "$COMPONENT_DIR/biophysical_neuron_models"
  },
  "node_sets_file": "$BASE_DIR/node_sets.json",
  "networks": {
    "nodes": [
      {
        "nodes_file": "$NETWORK_DIR/nodes1.h5",
        "node_types_file": null
      }
    ],
    "edges": [
      {
        "edges_file": "$NETWORK_DIR/edges1.h5",
        "edge_types_file": null
      }
    ]
  }
}

In this case, after replacing, the paths would be still relative, since the manifest variables are as well.

@mgeplf
Copy link
Contributor

mgeplf commented Jul 27, 2021

Yeah, I asked @joni-herttuainen to open this in case we wanted to discuss whether it should return an absolute path or not.

I can see the benefits for both, and the downsides; I'm not sure what the best solution is though. Having an extra argument to return_absolute_paths doesn't seem right.

However, pushing resolution down the library consumer is annoying - I doubt that the pwd when reading the files contained in the config is set to the same dir as the config, so relative paths will have to be manipulated and how best to address this is the goal of the ticket.

How are you handling this @NadirRoGue?

@NadirRoGue
Copy link
Contributor

NadirRoGue commented Jul 27, 2021

I have prepared the fix for this as well in case its decided to be commited. I only have to modify the nlohman::json object everytime a path is converted to absolute during parsing, and the returned paths by the expand JSON function returns the absolute ones.

I can open a PR if needed.

@mgeplf
Copy link
Contributor

mgeplf commented Jul 27, 2021

Wow, that's cool; if you already have something, I wouldn't mind seeing it - if you could push a branch, that would be helpful. Doesn't have to be polished or anything, just to get a sense of how it would work.

@NadirRoGue
Copy link
Contributor

NadirRoGue commented Jul 27, 2021

Here is the branch: https://github.com/BlueBrain/libsonata/tree/config_file_not_expanding
(edit: its a bad branch name for what it fixed, apologies for that)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants