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

Prepare chunk path format fix #356

Merged
merged 9 commits into from
Oct 27, 2023

Conversation

ducku
Copy link
Collaborator

@ducku ducku commented Oct 16, 2023

Noticed that using a bed url from the current repository gives a JSON parsing error, as well as a "path not allowed" error.

  • Wrapped track.json in an array to fix parsing error
  • Include path in track.json in compliance with recent internal dataPath changes

Copy link
Member

@adamnovak adamnovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it probably does make sense to switch to tracks.json having an array, instead of several objects in a row. It's less astonishing and people's editors like it better.

But to do that, I think we also need to:

  • Adjust the other example tracks.json files in the repo to add the [] and commas.
  • Get rid of the module we pull in for JSONParser, since it wouldn't be needed anymore.
  • Make corresponding changes to the prepare_local_chunk.sh script (or work out a way for it to share code so the changes can be made once).

# construct track JSON for graph file
jq -n --arg trackFile "${GRAPH_FILE}" --arg trackType "graph" --argjson trackColorSettings '{"mainPalette": "plainColors", "auxPalette": "greys"}' '$ARGS.named' >> $OUTDIR/tracks.json
GRAPH_FILE_PATH=$(realpath --relative-to ../ $GRAPH_FILE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't ../ here just the directory above the directory the user is standing in when they run the script? If they do something like:

TUBE_MAP_DIR="$(pwd)"
cd exampleData
mkdir project1
cd project1
mkdir dataset1
cd dataset1
wget https://random-graph.info/graph.xg
${TUBE_MAP_DIR}/scripts/prepare_chunk.sh -x graph.xg -r chr1:1000-2000 -o region1 >graph.bed

Then I don't think this would work.

We might want to get the script's own path, and use that to get the root directory of the tube map repo, and make the path relative to that? Or we could make the path relative to wherever tracks.json is put, and make the server resolve the path relative to where tracks.json is.

Copy link
Member

@adamnovak adamnovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks almost right, but it doesn't look like we should remove test_prechunk without editing cactus.bed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the whole exampleData/test_prechunk directory is being removed, but it is still referenced by cactus.bed. Either it should be retained and updated to the new format, or cactus.bed should be changed to point to a different place for that example chunk that is meant to exist.

@adamnovak adamnovak merged commit ede0f70 into vgteam:master Oct 27, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants