Skip to content

Commit

Permalink
Fixed nasty problem with z axis range set to null
Browse files Browse the repository at this point in the history
  • Loading branch information
ceriottm committed Nov 6, 2024
1 parent dcb7fd2 commit 21ddf7a
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 19 deletions.
1 change: 0 additions & 1 deletion python/chemiscope/jupyter.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ def show(
# and error is only visible in the console
if len(dict_input["properties"]) < 2:
raise ValueError("Need at least two properties to visualize a map widget")

return widget_class(dict_input, has_metadata=has_metadata)


Expand Down
4 changes: 2 additions & 2 deletions python/jupyter/src/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ChemiscopeBaseView extends DOMWidgetView {
}

const settings = this.model.get('settings') as Partial<Settings>;

// ignore pinned setting in jupyter, otherwise the pinned is changed
// by JS and then overwritten the first time by Python
delete settings.pinned;
Expand All @@ -51,7 +51,7 @@ class ChemiscopeBaseView extends DOMWidgetView {
}

protected _updatePythonSettings(): void {
if (this.visualizer !== undefined) {
if (this.visualizer !== undefined) {
const settings = this.visualizer.saveSettings();
// ignore pinned setting in jupyter, otherwise the pinned is changed
// by JS and then overwritten the first time by Python
Expand Down
24 changes: 12 additions & 12 deletions src/map/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,17 @@ export class MapOptions extends OptionsGroup {
super();

// Setup axes
const propertiesName = Object.keys(properties);
if (propertiesName.length < 2) {
const propertiesNames = Object.keys(properties);
if (propertiesNames.length < 2) {
throw new Error(
'Cannot show a map because the dataset contains fewer than two properties.'
);
}
this.x = new AxisOptions(propertiesName);
this.y = new AxisOptions(propertiesName);
this.x = new AxisOptions(propertiesNames);
this.y = new AxisOptions(propertiesNames);
// For z and color '' is a valid value
this.z = new AxisOptions(propertiesName.concat(['']));

this.z = new AxisOptions(propertiesNames.concat(['']));
console.log('initialized z as', this.z);

Check failure on line 112 in src/map/options.ts

View workflow job for this annotation

GitHub Actions / npm-test

Unexpected console statement
// Initialise symbol
this.symbol = new HTMLOption('string', '');
const validSymbols = [''];
Expand All @@ -131,7 +131,7 @@ export class MapOptions extends OptionsGroup {
min: new HTMLOption('number', NaN),
max: new HTMLOption('number', NaN),
};
this.color.property.validate = optionValidator(propertiesName.concat(['']), 'color');
this.color.property.validate = optionValidator(propertiesNames.concat(['']), 'color');
this.color.mode.validate = optionValidator(['linear', 'log', 'sqrt', 'inverse'], 'mode');

// Initialise size
Expand All @@ -140,7 +140,7 @@ export class MapOptions extends OptionsGroup {
mode: new HTMLOption('string', 'linear'),
property: new HTMLOption('string', ''),
};
this.size.property.validate = optionValidator(propertiesName.concat(['']), 'size');
this.size.property.validate = optionValidator(propertiesNames.concat(['']), 'size');
this.size.factor.validate = (value) => {
if (value < 1 || value > 100) {
throw Error(`size factor must be between 0 and 100, got ${value}`);
Expand All @@ -156,11 +156,11 @@ export class MapOptions extends OptionsGroup {
this.joinPoints = new HTMLOption('boolean', false);

// Setup default values
this.x.property.value = propertiesName[0];
this.y.property.value = propertiesName[1];
this.x.property.value = propertiesNames[0];
this.y.property.value = propertiesNames[1];
this.z.property.value = '';
if (propertiesName.length > 2) {
this.color.property.value = propertiesName[2];
if (propertiesNames.length > 2) {
this.color.property.value = propertiesNames[2];
} else {
this.color.property.value = '';
}
Expand Down
9 changes: 6 additions & 3 deletions src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ export abstract class OptionsGroup {
public applySettings(settings: Settings): void {
// make a copy of the settings since we will be changing it below
const copy = JSON.parse(JSON.stringify(settings)) as Settings;

this.foreachOption((keys, option) => {
/* eslint-disable */
assert(keys.length >= 1);
Expand All @@ -374,10 +373,14 @@ export abstract class OptionsGroup {
const lastKey = keys[keys.length - 1];

if (lastKey in root) {
const value = root[lastKey];
var value = root[lastKey];

// convert null values for numeric options to NaN (useful for axis range)
if (typeof option.value === 'number' && value === null) {
value = NaN;
}
// send warning if the value is invalid and omit applying it
if (value === undefined || value === null || Number.isNaN(value)) {
if (value === undefined || value === null) {
sendWarning(`ignored setting '${lastKey}' with invalid value '${value}'`);
return;
}
Expand Down
4 changes: 3 additions & 1 deletion src/structure/viewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,9 @@ export class MoleculeViewer {
return;
}

const active = this._environments.filter((e) => e !== undefined).map((e) => e.center);
const active = this._environments
.filter((e): e is Environment => e !== undefined)
.map((e) => e.center);

// resets clickable and hoverable state interactions
for (const atom of this._viewer.selectedAtoms({})) {
Expand Down

0 comments on commit 21ddf7a

Please sign in to comment.