Skip to content

Commit

Permalink
Merge pull request #721 from NeurodataWithoutBorders/fix-ophys-table-…
Browse files Browse the repository at this point in the history
…popup-and-validation

Fix Ophys Table Popup
  • Loading branch information
CodyCBakerPhD authored Apr 3, 2024
2 parents b3a3954 + 144ef2f commit 381f48e
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 29 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/pyflask-build-and-dist-tests.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Build Tests — Flask
name: Build Tests
on:
schedule:
- cron: "0 16 * * *" # Daily at noon EST
Expand All @@ -15,7 +15,7 @@ env:

jobs:
testing:
name: PyFlask build and distributable tests on ${{ matrix.os }}
name: PyInstaller on ${{ matrix.os }} # Will read on the dashboard as 'Build Tests / PyInstaller on {os}'
runs-on: ${{ matrix.os }}
defaults:
run:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/testing-live-services.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Dev Tests (Live Services)
name: Dev Tests (Live)
on:
schedule:
- cron: "0 16 * * *" # Daily at noon EST
Expand All @@ -13,7 +13,7 @@ env:

jobs:
testing:
name: Dev tests with live services on ${{ matrix.os }}
name: Services on ${{ matrix.os }} # Will read on the dashboard as 'Dev Tests (Live) / Services on {os}'
runs-on: ${{ matrix.os }}
defaults:
run:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ env:

jobs:
testing:
name: Dev tests on ${{ matrix.os }}
name: ${{ matrix.os }} # Will read on the dashboard as 'Dev Tests / {os}'
runs-on: ${{ matrix.os }}
defaults:
run:
Expand Down
15 changes: 14 additions & 1 deletion schemas/base-metadata.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { header, replaceRefsWithValue } from '../src/renderer/src/stories/forms/
import baseMetadataSchema from './json/base_metadata_schema.json' assert { type: "json" }

import { merge } from '../src/renderer/src/stories/pages/utils'
import { drillSchemaProperties } from '../src/renderer/src/stories/pages/guided-mode/data/utils'

const UV_MATH_FORMAT = `&micro;V`; //`<math xmlns="http://www.w3.org/1998/Math/MathML"><mo>&micro;</mo><mi>V</mi></math>`
const UV_PROPERTIES = ["gain_to_uV", "offset_to_uV"]
Expand Down Expand Up @@ -71,8 +72,20 @@ export const preprocessMetadataSchema = (schema: any = baseMetadataSchema, globa

const copy = replaceRefsWithValue(structuredClone(schema))

copy.additionalProperties = false
// NEUROCONV PATCH: Correct for incorrect array schema
drillSchemaProperties(
copy,
(_, schema) => {
if (schema.properties && schema.type === "array") {
schema.items = { type: "object", properties: schema.properties, required: schema.required };
delete schema.properties;
delete schema.required;
}
},
{}
);

copy.additionalProperties = false


copy.required = Object.keys(copy.properties) // Require all properties at the top level
Expand Down
2 changes: 0 additions & 2 deletions src/renderer/src/stories/InstanceManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ export class InstanceManager extends LitElement {

// Correct bug where multiple instances are selected
updated = () => {
console.log(this);
const selected = Array.from(this.shadowRoot.querySelectorAll("[selected]"));
if (selected.length > 0)
selected.slice(1).forEach((element) => {
Expand Down Expand Up @@ -376,7 +375,6 @@ export class InstanceManager extends LitElement {
this.#items = [];

const instances = this.#render();
console.log(this.#items);

const hasMultiple = this.#hasMultiple();

Expand Down
5 changes: 3 additions & 2 deletions src/renderer/src/stories/JSONSchemaForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,9 @@ export class JSONSchemaForm extends LitElement {
if (e.message.includes("is not of a type(s)")) {
if (resolvedSchema.type === "number") {
if (resolvedValue === "NaN") return;
else if (resolvedValue === null) return;
else if (isRow) e.message = `${e.message}. ${templateNaNMessage}`;
else if (resolvedValue === null) {
if (schema.type !== "array") return; // Allow null in non-tables
} else if (isRow) e.message = `${e.message}. ${templateNaNMessage}`;
} else if (resolvedSchema.type === "string") {
if ("properties" in resolvedSchema) return; // Allow for constructing types from object types
}
Expand Down
5 changes: 1 addition & 4 deletions src/renderer/src/stories/JSONSchemaInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ export function createTable(fullPath, { onUpdate, onThrow, overrides = {} }) {

merge(overrides.schema, schemaCopy, { arrays: true });

console.log(schemaPath, nestedIgnore);

const tableMetadata = {
keyColumn: tempPropertyKey,
schema: schemaCopy,
Expand Down Expand Up @@ -928,7 +926,6 @@ export class JSONSchemaInput extends LitElement {
const allowPatternProperties = isPatternProperties(this.pattern);
const allowAdditionalProperties = isAdditionalProperties(this.pattern);

// Provide default item types
// Provide default item types
if (isArray) {
const hasItemsRef = "items" in schema && "$ref" in schema.items;
Expand Down Expand Up @@ -1016,7 +1013,7 @@ export class JSONSchemaInput extends LitElement {
});
}

const externalPath = this.form ? [...this.form.base, ...resolvedFullPath] : resolvedFullPath;
const externalPath = this.form?.base ? [...this.form.base, ...resolvedFullPath] : resolvedFullPath;

const table = createTable.call(this, externalPath, {
onUpdate: updateFunction,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ export class GuidedMetadataPage extends ManagedPage {

const ophys = schema.properties.Ophys;
if (ophys) {
// Set most Ophys tables to have minItems / maxItems equal (i.e. no editing possible)
drillSchemaProperties(
schema,
(path, schema, target, isPatternProperties, parentSchema) => {
Expand All @@ -258,11 +257,14 @@ export class GuidedMetadataPage extends ManagedPage {

if (schema.type === "array") {
if (name !== "Device" && target) {
if (name in target)
schema.minItems = schema.maxItems = target[name].length; // Skip unresolved deep in pattern properties)
// Remove Ophys requirements if left initially undefined
else if (parentSchema.required.includes(name))
parentSchema.required = parentSchema.required.filter((n) => n !== name);
// Set most Ophys tables to have minItems / maxItems equal (i.e. no editing possible)
if (name in target) schema.minItems = schema.maxItems = target[name].length;
// Remove Ophys property requirement if left initially undefined
else {
target[name] = []; // Initialize empty array
if (parentSchema.required.includes(name))
parentSchema.required = parentSchema.required.filter((n) => n !== name);
}
}
}
}
Expand All @@ -272,6 +274,7 @@ export class GuidedMetadataPage extends ManagedPage {
}

console.log("schema", structuredClone(schema), structuredClone(results));

// Create the form
const form = new JSONSchemaForm({
identifier: instanceId,
Expand Down
6 changes: 0 additions & 6 deletions src/renderer/src/stories/pages/guided-mode/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ export function resolveProperties(properties = {}, target, globals = {}) {
for (let name in properties) {
const info = properties[name];

// NEUROCONV PATCH: Correct for incorrect array schema
if (info.properties && info.type === "array") {
info.items = { type: "object", properties: info.properties, required: info.required };
delete info.properties;
}

const props = info.properties;

if (!(name in target)) {
Expand Down
1 change: 0 additions & 1 deletion src/renderer/src/stories/table/cells/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ export class TableCellBase extends LitElement {
this.setAttribute('editing', '')

const listenForEnter = (ev: KeyboardEvent) => {
console.log(ev)

if (ev.key === 'Enter') {
ev.preventDefault()
Expand Down
7 changes: 5 additions & 2 deletions src/renderer/src/stories/table/cells/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export class NestedEditor extends LitElement {
const schema = this.schema


console.log('schema', schema, 'data', data)
const container = document.createElement('div')
const input = this.#input = new JSONSchemaInput({
schema,
value: data,
Expand All @@ -59,9 +61,10 @@ export class NestedEditor extends LitElement {
renderTable: (name, metadata, path) => new SimpleTable(metadata) // NOTE: Would be most ideal to have a reference to the containing input...
})

input.style.padding = '25px 50px'
container.style.padding = '25px'
container.append(input)

modal.append(input)
modal.append(container)

modal.open = true
}
Expand Down

0 comments on commit 381f48e

Please sign in to comment.