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

Fix download empty tree #1591

Merged
merged 3 commits into from
Nov 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/components/download/downloadButtons.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ export const DownloadButtons = ({dispatch, t, tree, entropy, metadata, colorBy,
Downloaded data represents the currently displayed view.
By zooming the tree, changing the branch-length metric, applying filters etc, the downloaded data will change accordingly.
<p/>
NOTE: We do not support downloads of multiple subtrees, which are usually created with the date range filter or genotype filters.
Downloading multiple subtrees will result in an empty Newick tree!
<p/>
{partialData ? `Currently ${selectedTipsCount}/${totalTipCount} tips are displayed and will be downloaded.` : `Currently the entire dataset (${totalTipCount} tips) will be downloaded.`}
</div>
<Button
Expand Down
20 changes: 17 additions & 3 deletions src/components/download/helperFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { numericToCalendar } from "../../util/dateHelpers";
import { NODE_VISIBLE } from "../../util/globals";
import { datasetSummary } from "../info/datasetSummary";
import { isColorByGenotype } from "../../util/getGenotype";
import { EmptyNewickTreeCreated } from "../../util/exceptions";

export const isPaperURLValid = (d) => {
return (
Expand Down Expand Up @@ -36,9 +37,18 @@ const treeToNewick = (tree, temporal, internalNodeNames=false, nodeAnnotation=()
return leaf;
}

const rootNode = tree.nodes[tree.idxOfInViewRootNode];
/**
* Try the filtered root first as this may be different from the in view root node
* We still need to fallback on the idxOfInViewRootNode because the idxOfFilteredRoot
* is undefined when there are no filters applied.
*/
const rootNode = tree.nodes[tree.idxOfFilteredRoot || tree.idxOfInViewRootNode];
const rootXVal = getXVal(rootNode);
return recurse(rootNode, rootXVal) + ";";
const newickTree = recurse(rootNode, rootXVal);
if (!newickTree) {
throw new EmptyNewickTreeCreated();
}
return newickTree + ";";
};

const MIME = {
Expand Down Expand Up @@ -329,7 +339,11 @@ export const exportTree = ({dispatch, filePrefix, tree, isNewick, temporal, colo
dispatch(infoNotification({message: `${temporal ? "TimeTree" : "Tree"} written to ${fName}`}));
} catch (err) {
console.error(err);
dispatch(warningNotification({message: "Error saving tree!"}));
const warningObject = {message: "Error saving tree!"};
if (err instanceof EmptyNewickTreeCreated) {
warningObject.details = "An empty tree was created. If you have selected genomes, note that we do not support downloads of multiple subtrees.";
}
dispatch(warningNotification(warningObject));
}
};

Expand Down
12 changes: 12 additions & 0 deletions src/util/exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,15 @@ export class NoContentError extends Error {
super(...params);
}
}

/**
* Thrown when a download produces an empty Newick tree.
* Usually caused by users trying to download multiple subtrees that do not
* share a common ancestor that is "visible"
*/
export class EmptyNewickTreeCreated extends Error {
constructor(...params) {
super(...params);
this.name = this.constructor.name;
}
}