From 7762b61904e6c3b40603918fc23e79073066c360 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Tue, 20 Feb 2024 06:27:42 +0900 Subject: [PATCH 1/2] Make interface display names more human readable (#589) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Garrett Michael Flynn Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> --- pyflask/manageNeuroconv/manage_neuroconv.py | 21 ++++++-- pyflask/tests/test_neuroconv.py | 4 +- src/renderer/src/stories/Search.js | 54 +++++++++++++++---- .../pages/guided-mode/data/GuidedStructure.js | 21 +++++--- tests/e2e.test.ts | 2 +- 5 files changed, 79 insertions(+), 23 deletions(-) diff --git a/pyflask/manageNeuroconv/manage_neuroconv.py b/pyflask/manageNeuroconv/manage_neuroconv.py index 79e7caf58..3e93fefc2 100644 --- a/pyflask/manageNeuroconv/manage_neuroconv.py +++ b/pyflask/manageNeuroconv/manage_neuroconv.py @@ -163,19 +163,32 @@ def get_class_ref_in_docstring(input_string): def derive_interface_info(interface): + info = {"keywords": getattr(interface, "keywords", []), "description": ""} - if interface.__doc__: + + if hasattr(interface, "associated_suffixes"): + info["suffixes"] = interface.associated_suffixes + + if hasattr(interface, "info"): + info["description"] = interface.info + + elif interface.__doc__: info["description"] = re.sub( remove_extra_spaces_pattern, " ", re.sub(doc_pattern, r"\1", interface.__doc__) ) + info["name"] = interface.__name__ + return info def get_all_converter_info() -> dict: - from neuroconv import converters + from neuroconv.converters import converter_list - return {name: derive_interface_info(converter) for name, converter in module_to_dict(converters).items()} + return { + getattr(converter, "display_name", converter.__name__) or converter.__name__: derive_interface_info(converter) + for converter in converter_list + } def get_all_interface_info() -> dict: @@ -201,7 +214,7 @@ def get_all_interface_info() -> dict: ] return { - interface.__name__: derive_interface_info(interface) + getattr(interface, "display_name", interface.__name__) or interface.__name__: derive_interface_info(interface) for interface in interface_list if not interface.__name__ in exclude_interfaces_from_selection } diff --git a/pyflask/tests/test_neuroconv.py b/pyflask/tests/test_neuroconv.py index a2c8228dd..7e6bd33e8 100644 --- a/pyflask/tests/test_neuroconv.py +++ b/pyflask/tests/test_neuroconv.py @@ -12,12 +12,14 @@ def test_get_all_interfaces(client): "^.*Interface$": { "type": "object", "properties": { + "name": {"type": "string"}, + "suffixes": {"type": "array", "items": {"type": "string"}}, "label": {"type": "string"}, "description": {"type": "string"}, "keywords": {"type": "array", "items": {"type": "string"}}, }, "additionalProperties": False, - "required": ["keywords"], + "required": ["name", "keywords"], } }, }, diff --git a/src/renderer/src/stories/Search.js b/src/renderer/src/stories/Search.js index 2e75a01e6..50cd073aa 100644 --- a/src/renderer/src/stories/Search.js +++ b/src/renderer/src/stories/Search.js @@ -158,6 +158,11 @@ export class Search extends LitElement { z-index: 1; } + .structured-keywords { + font-size: 90%; + color: dimgrey; + } + .option:hover { background: #f2f2f2; cursor: pointer; @@ -204,8 +209,12 @@ export class Search extends LitElement { const options = this.shadowRoot.querySelectorAll(".option"); this.#options = Array.from(options).map((option) => { const keywordString = option.getAttribute("data-keywords"); - const keywords = keywordString ? JSON.parse(keywordString) : []; - return { option, keywords, label: option.querySelector(".label").innerText }; + const structuredKeywordString = option.getAttribute("data-structured-keywords"); + + const keywords = keywordString ? JSON.parse(option.getAttribute("data-keywords")) : []; + const structuredKeywords = structuredKeywordString ? JSON.parse(structuredKeywordString) : {}; + + return { option, keywords, structuredKeywords, label: option.querySelector(".label").innerText }; }); this.#initialize(); @@ -256,8 +265,8 @@ export class Search extends LitElement { }); // Check if the input value matches any of the keywords - this.#options.forEach(({ option, keywords = [] }, i) => { - keywords.forEach((keyword) => { + this.#options.forEach(({ option, keywords = [], structuredKeywords = {} }, i) => { + [...keywords, ...Object.values(structuredKeywords).flat()].forEach((keyword) => { if (keyword.toLowerCase().includes(input.toLowerCase()) && !toShow.includes(i)) toShow.push(i); }); }); @@ -317,13 +326,22 @@ export class Search extends LitElement { listItemElement.classList.add("option"); listItemElement.setAttribute("hidden", ""); listItemElement.setAttribute("tabindex", -1); - if (option.keywords) listItemElement.setAttribute("data-keywords", JSON.stringify(option.keywords)); + + const { disabled, structuredKeywords, keywords } = option; + + if (structuredKeywords) + listItemElement.setAttribute( + "data-structured-keywords", + JSON.stringify(option.structuredKeywords) + ); + if (keywords) listItemElement.setAttribute("data-keywords", JSON.stringify(option.keywords)); + listItemElement.addEventListener("click", (clickEvent) => { clickEvent.stopPropagation(); this.#onSelect(option); }); - if (option.disabled) listItemElement.setAttribute("disabled", ""); + if (disabled) listItemElement.setAttribute("disabled", ""); const container = document.createElement("div"); @@ -346,11 +364,25 @@ export class Search extends LitElement { container.appendChild(label); - if (option.keywords) { - const keywords = document.createElement("small"); - keywords.classList.add("keywords"); - keywords.innerText = option.keywords.join(", "); - container.appendChild(keywords); + if (keywords) { + const keywordsElement = document.createElement("small"); + keywordsElement.classList.add("keywords"); + keywordsElement.innerText = option.keywords.join(", "); + container.appendChild(keywordsElement); + } + + if (structuredKeywords) { + const div = document.createElement("div"); + div.classList.add("structured-keywords"); + + Object.entries(structuredKeywords).forEach(([key, value]) => { + const keywordsElement = document.createElement("small"); + const capitalizedKey = key[0].toUpperCase() + key.slice(1); + keywordsElement.innerHTML = `${capitalizedKey}: ${value.join(", ")}`; + div.appendChild(keywordsElement); + }); + + container.appendChild(div); } listItemElement.append(container); diff --git a/src/renderer/src/stories/pages/guided-mode/data/GuidedStructure.js b/src/renderer/src/stories/pages/guided-mode/data/GuidedStructure.js index 0d8a72694..584c5f708 100644 --- a/src/renderer/src/stories/pages/guided-mode/data/GuidedStructure.js +++ b/src/renderer/src/stories/pages/guided-mode/data/GuidedStructure.js @@ -115,15 +115,24 @@ export class GuidedStructurePage extends Page { .then((res) => res.json()) .then((json) => Object.entries(json).map(([key, value]) => { - const category = categories.find(({ test }) => test.test(key))?.value; + const displayName = key.trim(); + + const interfaceName = value.name; + + const category = categories.find(({ test }) => test.test(interfaceName))?.value; + + const structuredKeywords = { + suffixes: value.suffixes ?? [], + }; return { - ...value, - key, - value: key, + ...value, // Contains label and name already (extra metadata) + key: displayName, + value: interfaceName, + structuredKeywords, category, - disabled: !supportedInterfaces.includes(key), - }; // Has label and keywords property already + disabled: !supportedInterfaces.includes(interfaceName), + }; }) ) .catch((error) => console.error(error)); diff --git a/tests/e2e.test.ts b/tests/e2e.test.ts index 336b972ee..21b18880b 100644 --- a/tests/e2e.test.ts +++ b/tests/e2e.test.ts @@ -342,7 +342,7 @@ describe('E2E Test', () => { await toNextPage('inspect') - }, 30 * 1000) // Wait for conversion to complete + }, 30 * 1000) // Wait for conversion preview to complete test('Review NWB Inspector output', async () => { From d80764a79224b0c6b6321830abf06b2c505d15ca Mon Sep 17 00:00:00 2001 From: Garrett Michael Flynn Date: Tue, 20 Feb 2024 11:07:28 -0600 Subject: [PATCH 2/2] Improved Path Expansion Interaction (#609) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> --- pyflask/apis/neuroconv.py | 13 +- pyflask/manageNeuroconv/__init__.py | 1 + pyflask/manageNeuroconv/manage_neuroconv.py | 47 +++++- .../src/stories/FileSystemSelector.js | 10 -- src/renderer/src/stories/JSONSchemaForm.js | 2 + src/renderer/src/stories/JSONSchemaInput.js | 2 +- .../guided-mode/data/GuidedPathExpansion.js | 156 +++++++++++++++++- 7 files changed, 217 insertions(+), 14 deletions(-) diff --git a/pyflask/apis/neuroconv.py b/pyflask/apis/neuroconv.py index 8b780c11e..62028e365 100644 --- a/pyflask/apis/neuroconv.py +++ b/pyflask/apis/neuroconv.py @@ -9,12 +9,12 @@ get_all_interface_info, get_all_converter_info, locate_data, + autocomplete_format_string, get_source_schema, get_metadata_schema, convert_to_nwb, validate_metadata, listen_to_neuroconv_events, - generate_dataset, inspect_nwb_file, inspect_nwb_folder, inspect_multiple_filesystem_objects, @@ -78,6 +78,17 @@ def post(self): neuroconv_api.abort(500, str(exception)) +@neuroconv_api.route("/locate/autocomplete") +class Locate(Resource): + @neuroconv_api.doc(responses={200: "Success", 400: "Bad Request", 500: "Internal server error"}) + def post(self): + try: + return autocomplete_format_string(neuroconv_api.payload) + except Exception as exception: + if notBadRequestException(exception): + neuroconv_api.abort(500, str(exception)) + + @neuroconv_api.route("/metadata") class Metadata(Resource): @neuroconv_api.doc(responses={200: "Success", 400: "Bad Request", 500: "Internal server error"}) diff --git a/pyflask/manageNeuroconv/__init__.py b/pyflask/manageNeuroconv/__init__.py index aca8f4125..ddcc2b8b6 100644 --- a/pyflask/manageNeuroconv/__init__.py +++ b/pyflask/manageNeuroconv/__init__.py @@ -2,6 +2,7 @@ get_all_interface_info, get_all_converter_info, locate_data, + autocomplete_format_string, get_source_schema, get_metadata_schema, convert_to_nwb, diff --git a/pyflask/manageNeuroconv/manage_neuroconv.py b/pyflask/manageNeuroconv/manage_neuroconv.py index 3e93fefc2..a47a94db9 100644 --- a/pyflask/manageNeuroconv/manage_neuroconv.py +++ b/pyflask/manageNeuroconv/manage_neuroconv.py @@ -18,6 +18,18 @@ announcer = MessageAnnouncer() +def is_path_contained(child, parent): + parent = Path(parent) + child = Path(child) + + # Attempt to construct a relative path from parent to child + try: + child.relative_to(parent) + return True + except ValueError: + return False + + def replace_nan_with_none(data): if isinstance(data, dict): # If it's a dictionary, iterate over its items and replace NaN values with None @@ -109,6 +121,39 @@ def coerce_schema_compliance_recursive(obj, schema): ) +def autocomplete_format_string(info: dict) -> str: + from neuroconv.tools.path_expansion import construct_path_template + + base_directory = info["base_directory"] + filesystem_entry_path = info["path"] + + if not is_path_contained(filesystem_entry_path, base_directory): + raise ValueError("Path is not contained in the provided base directory.") + + full_format_string = construct_path_template( + filesystem_entry_path, + subject_id=info["subject_id"], + session_id=info["session_id"], + **info["additional_metadata"], + ) + + parent = Path(base_directory).resolve() + child = Path(full_format_string).resolve() + + format_string = str(child.relative_to(parent)) + + to_locate_info = dict(base_directory=base_directory) + + if Path(filesystem_entry_path).is_dir(): + to_locate_info["folder_path"] = format_string + else: + to_locate_info["file_path"] = format_string + + all_matched = locate_data(dict(autocomplete=to_locate_info)) + + return dict(matched=all_matched, format_string=format_string) + + def locate_data(info: dict) -> dict: """Locate data from the specifies directories using fstrings.""" from neuroconv.tools import LocalPathExpander @@ -622,7 +667,7 @@ def generate_dataset(input_path: str, output_path: str): if base_id in file: os.rename(os.path.join(root, file), os.path.join(root, file.replace(base_id, full_id))) - phy_output_dir.symlink_to(phy_base_directory, True) + copytree(phy_base_directory, phy_output_dir) return {"output_path": str(output_path)} diff --git a/src/renderer/src/stories/FileSystemSelector.js b/src/renderer/src/stories/FileSystemSelector.js index 831fbe236..ec4df9189 100644 --- a/src/renderer/src/stories/FileSystemSelector.js +++ b/src/renderer/src/stories/FileSystemSelector.js @@ -187,16 +187,6 @@ export class FilesystemSelector extends LitElement { const len = isArray ? this.value.length : 0; - if (isArray) { - resolved = this.value.map((str) => str.replaceAll("\\", "/")); - isUpdated = JSON.stringify(resolved) !== JSON.stringify(this.value); - } else { - resolved = typeof this.value === "string" ? this.value.replaceAll("\\", "/") : this.value; - isUpdated = resolved !== this.value; - } - - if (isUpdated) this.#handleFiles(resolved); // Notify of the change to the separators - const resolvedValueDisplay = isArray ? len > 1 ? `${this.value[0]} and ${len - 1} other${len > 2 ? "s" : ""}` diff --git a/src/renderer/src/stories/JSONSchemaForm.js b/src/renderer/src/stories/JSONSchemaForm.js index 7bcca50af..c08b8c1ff 100644 --- a/src/renderer/src/stories/JSONSchemaForm.js +++ b/src/renderer/src/stories/JSONSchemaForm.js @@ -1110,6 +1110,8 @@ export class JSONSchemaForm extends LitElement { results: { ...nestedResults }, globals: this.globals?.[name], + controls: this.controls[name], + onUpdate: (internalPath, value, forceUpdate) => { const path = [...localPath, ...internalPath]; this.updateData(path, value, forceUpdate); diff --git a/src/renderer/src/stories/JSONSchemaInput.js b/src/renderer/src/stories/JSONSchemaInput.js index 2ba7acea7..281c281df 100644 --- a/src/renderer/src/stories/JSONSchemaInput.js +++ b/src/renderer/src/stories/JSONSchemaInput.js @@ -288,7 +288,7 @@ export const isEditableObject = (schema, value) => export const isAdditionalProperties = (pattern) => pattern === "additional"; export const isPatternProperties = (pattern) => pattern && !isAdditionalProperties(pattern); -export const getEditableItems = (value, pattern, { name, schema } = {}) => { +export const getEditableItems = (value = {}, pattern, { name, schema } = {}) => { let items = Object.entries(value); const allowAdditionalProperties = isAdditionalProperties(pattern); diff --git a/src/renderer/src/stories/pages/guided-mode/data/GuidedPathExpansion.js b/src/renderer/src/stories/pages/guided-mode/data/GuidedPathExpansion.js index 1de2bd33d..30380b8bc 100644 --- a/src/renderer/src/stories/pages/guided-mode/data/GuidedPathExpansion.js +++ b/src/renderer/src/stories/pages/guided-mode/data/GuidedPathExpansion.js @@ -14,6 +14,143 @@ import { CodeBlock } from "../../../CodeBlock.js"; import { List } from "../../../List"; import { fs } from "../../../../electron/index.js"; import { joinPath } from "../../../../globals.js"; +import { Button } from "../../../Button.js"; +import { Modal } from "../../../Modal"; +import { header } from "../../../forms/utils"; + +import autocompleteIcon from "../../../assets/inspect.svg?raw"; + +export async function autocompleteFormatString(path) { + let notification; + + const { base_directory } = path.reduce((acc, key) => acc[key] ?? {}, this.form.resolved); + + const notify = (message, type) => { + if (notification) this.dismiss(notification); + return (notification = this.notify(message, type)); + }; + + if (!base_directory) { + const message = `Please fill out the base directory for ${header(path[0])} before attempting auto-completion.`; + notify(message, "error"); + throw new Error(message); + } + + const modal = new Modal({ + header: "Autocomplete Format String", + }); + + const content = document.createElement("div"); + Object.assign(content.style, { + padding: "25px", + paddingBottom: "0px", + }); + + const propOrder = ["path", "subject_id", "session_id"]; + const form = new JSONSchemaForm({ + schema: { + type: "object", + properties: { + path: { + type: "string", + title: "Example Filesystem Entry", + format: ["file", "directory"], + description: "Provide an example filesystem entry for the selected interface", + }, + subject_id: { + type: "string", + description: "The subject ID in the above entry", + }, + session_id: { + type: "string", + description: "The session ID in the above entry", + }, + }, + required: propOrder, + order: propOrder, + }, + validateOnChange: async (name, parent) => { + const value = parent[name]; + + if (name === "path") { + if (value) { + if (fs.lstatSync(value).isSymbolicLink()) + return [ + { + type: "error", + message: "This feature does not support symbolic links. Please provide a valid path.", + }, + ]; + + if (base_directory) { + if (!value.includes(base_directory)) + return [ + { + type: "error", + message: + "The provided path must include the base directory.
This is likely due to the target being contained in a symlink, which is unsupported by this feature.", + }, + ]; + } + + const errors = []; + for (let key in parent) { + if (key === name) continue; + if (!value.includes(parent[key])) + errors.push({ + type: "error", + message: `${header(name)} not found in the updated path.`, + }); + } + } + } else { + if (!parent.path || !parent.path.includes(value)) + return [ + { + type: "error", + message: `${header(name)} not found in the provided path.`, + }, + ]; + } + }, + }); + + content.append(form); + modal.append(content); + + modal.onClose = async () => notify("Format String Path was not completed.", "error"); + + return new Promise((resolve) => { + const button = new Button({ + label: "Create", + primary: true, + onClick: async () => { + await form.validate().catch((e) => { + notify(e.message, "error"); + throw e; + }); + + const results = await run("locate/autocomplete", { + base_directory, + additional_metadata: {}, + ...form.results, + }); + const input = this.form.getFormElement([...path, "format_string_path"]); + input.updateData(results.format_string); + this.save(); + resolve(results.format_string); + }, + }); + + modal.footer = button; + + modal.open = true; + + document.body.append(modal); + }).finally(() => { + modal.remove(); + }); +} const exampleFileStructure = `mylab/ ¦ Subjects/ @@ -271,8 +408,23 @@ export class GuidedPathExpansionPage extends Page { // Require properties for all sources const generatedSchema = { type: "object", properties: {}, additionalProperties: false }; - for (let key in this.info.globalState.interfaces) + const controls = {}; + for (let key in this.info.globalState.interfaces) { generatedSchema.properties[key] = { type: "object", ...pathExpansionSchema }; + + controls[key] = { + format_string_path: [ + new Button({ + label: "Autocomplete", + icon: autocompleteIcon, + buttonStyles: { + width: "max-content", + }, + onClick: async () => autocompleteFormatString.call(this, [key]), + }), + ], + }; + } structureState.schema = generatedSchema; this.optional.requestUpdate(); @@ -282,6 +434,8 @@ export class GuidedPathExpansionPage extends Page { onThrow, validateEmptyValues: false, + controls, + // NOTE: These are custom coupled form inputs onUpdate: (path, value) => { this.unsavedUpdates = "conversions";