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

API Versioning + Migration #2430

Merged
merged 5 commits into from
Nov 17, 2023
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
25 changes: 19 additions & 6 deletions docs/docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ To install a specific version of Emscripten (e.g. `2.0.6`):

---

## `perspective-python`

To build the Python library, first configure your project to build Python via
`yarn setup`. Then, install the requirements corresponding to your version of
python, e.g.

```bash
pip install -r python/perspective/requirements-311.txt
```

`perspective-python` supports Python 3.8 and upwards.

Copy link
Member

Choose a reason for hiding this comment

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

yarn _requires_python is the correct way to do this. While this is not documented, a good place to look is always the CI actions yaml

### `perspective-jupyterlab`

To install the Jupyterlab/Jupyter Notebook plugins from your local working
Expand All @@ -108,17 +120,18 @@ Afterwards, you should see it listed as a "local extension" when you run
`jupyter labextension list` and as a normal extension when you run
`jupyter nbextension list`.

## `perspective-python`

To build the Python library, first configure your project to build Python via
`yarn setup`, then run:
As an example, your setup process might look like this:

```bash
python -m venv ./venv
pip install -r python/perspective/requirements-311.txt
yarn setup # choose python
yarn build
yarn setup # choose javascript > jupyterlab
yarn build
yarn jlab_link # run this whenever you need to update a local perspective package
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't actually work due to a bug. yarn jlab_link currently installs perspective-python in your virtual env, including the python and C++ extension, so further rebuilds in your source repo won't get picked up.

```

`perspective-python` supports Python 3.8 and upwards.

---

## System-Specific Instructions
Expand Down
10 changes: 9 additions & 1 deletion packages/perspective-jupyterlab/src/js/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ export class PerspectiveView extends DOMWidgetView {
typeof new_value === "string" &&
name !== "plugin" &&
name !== "theme" &&
name !== "title"
name !== "title" &&
name !== "version"
) {
new_value = JSON.parse(new_value);
}
Expand Down Expand Up @@ -270,6 +271,7 @@ export class PerspectiveView extends DOMWidgetView {
theme: this.model.get("theme"),
settings: this.model.get("settings"),
title: this.model.get("title"),
version: this.model.get("version"),
});
}

Expand Down Expand Up @@ -506,4 +508,10 @@ export class PerspectiveView extends DOMWidgetView {
title: this.model.get("title"),
});
}

version_changed() {
this.luminoWidget.restore({
version: this.model.get("version"),
});
}
}
8 changes: 6 additions & 2 deletions packages/perspective-jupyterlab/test/jupyter/widget.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ describe_jupyter(

// Check default config
expect(config).toEqual({
version: utils.API_VERSION,
aggregates: {},
columns: [
"ui8",
Expand Down Expand Up @@ -267,6 +268,7 @@ w.theme = "Pro Dark"`

// and check it
expect(config).toEqual({
version: utils.API_VERSION,
aggregates: {},
columns: ["ui8"],
expressions: [],
Expand Down Expand Up @@ -300,6 +302,7 @@ w.theme = "Pro Dark"`

// Check default config
expect(config).toEqual({
version: utils.API_VERSION,
aggregates: {},
columns: [
"ui8",
Expand Down Expand Up @@ -333,8 +336,9 @@ w.theme = "Pro Dark"`
title: null,
});

await viewer.evaluate(async (viewer) => {
await viewer.evaluate(async (viewer, version) => {
viewer.restore({
version,
columns: ["ui8"],
filter: [["i8", "<", "50"]],
group_by: ["date"],
Expand All @@ -347,7 +351,7 @@ w.theme = "Pro Dark"`
});

return "";
});
}, utils.API_VERSION);

const error_cells_dont_exist = await assert_no_error_in_cell(
page,
Expand Down
19 changes: 5 additions & 14 deletions packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,18 @@
// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

import * as fc from "d3fc";
import { select } from "d3";
import { axisFactory } from "../axis/axisFactory";
import { chartCanvasFactory } from "../axis/chartFactory";
import {
pointSeriesCanvas,
symbolTypeFromColumn,
} from "../series/pointSeriesCanvas";
import { symbolTypeFromColumn } from "../series/pointSeriesCanvas";
import { pointData } from "../data/pointData";
import {
seriesColorsFromColumn,
seriesColorsFromDistinct,
colorScale,
} from "../series/seriesColors";
import { seriesLinearRange, seriesColorRange } from "../series/seriesRange";
import { symbolLegend, colorLegend, colorGroupLegend } from "../legend/legend";
import { seriesColorRange } from "../series/seriesRange";
import { symbolLegend, colorLegend } from "../legend/legend";
import { colorRangeLegend } from "../legend/colorRangeLegend";
import { filterDataByGroup } from "../legend/filter";
import withGridLines from "../gridlines/gridlines";
import { hardLimitZeroPadding } from "../d3fc/padding/hardLimitZero";
import zoomableChart from "../zoom/zoomableChart";
import nearbyTip from "../tooltip/nearbyTip";
import { symbolsObj } from "../series/seriesSymbols";
import { gridLayoutMultiChart } from "../layout/gridLayoutMultiChart";
import xyScatterSeries from "../series/xy-scatter/xyScatterSeries";
Expand All @@ -55,7 +45,8 @@ function overrideSymbols(settings, symbols) {
for (let i in domain) {
range[i] = range[i % len];
}
settings.columns?.[symbolCol]?.symbols?.forEach(({ key, value }) => {
let maybeSymbols = settings.columns?.[symbolCol]?.symbols;
Object.entries(maybeSymbols ?? {}).forEach(([key, value]) => {
// TODO: Define custom symbol types based on the values passed in here.
// https://d3js.org/d3-shape/symbol#custom-symbols
let symbolType = symbolsObj[value] ?? d3.symbolCircle;
Expand Down
9 changes: 8 additions & 1 deletion packages/perspective-viewer-d3fc/src/less/chart.less
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@
padding: 0;
font-size: 14px;

d3fc-group:first-child {
padding: 16px;
padding-top: 0px;
width: calc(100% - 32px);
height: calc(100% - 16px);
}

& .multi-xlabel {
position: absolute;
bottom: 0;
Expand Down Expand Up @@ -87,7 +94,7 @@
& .inner-container {
display: inline-grid;
overflow-y: auto;
width: 100%;
width: calc(100% - 16px);
height: 100%;
padding: 0;
margin: 0;
Expand Down
6 changes: 5 additions & 1 deletion packages/perspective-viewer-d3fc/test/js/barWidth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

import { test, expect } from "@playwright/test";
import { compareSVGContentsToSnapshot } from "@finos/perspective-test";
import {
API_VERSION,
compareSVGContentsToSnapshot,
} from "@finos/perspective-test";

test.describe("Bar Width", () => {
test("correctly render when a bar chart has non equidistant times on a datetime axis", async ({
Expand All @@ -38,6 +41,7 @@ test.describe("Bar Width", () => {
);

expect(config).toEqual({
version: API_VERSION,
plugin: "Y Bar",
columns: ["Profit"],
group_by: ["Order Date"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function write_cell(table, model, active_cell) {
return false;
}
} else if (type === "boolean") {
text = text === "check" ? false : text === "close" ? true : null;
text = text === "true" ? false : text === "false" ? true : null;
}

const msg = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
const { convert } = require("@finos/perspective-viewer/dist/cjs/migrate.js");
import { test, expect } from "@playwright/test";
import {
API_VERSION,
compareLightDOMContents,
compareShadowDOMContents,
} from "@finos/perspective-test";
Expand Down Expand Up @@ -60,6 +61,7 @@ const TESTS = [
{
viewers: {
One: {
version: API_VERSION,
table: "superstore",
title: "One",
plugin: "Y Area",
Expand Down
1 change: 1 addition & 0 deletions packages/perspective/src/js/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const CONFIG_ALIASES = {
};

export const CONFIG_VALID_KEYS = [
"version",
"viewport",
"group_by",
"split_by",
Expand Down
11 changes: 10 additions & 1 deletion python/perspective/perspective/tests/viewer/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from perspective.core import PerspectiveError
from perspective.core import Plugin
import perspective.viewer.validate as validate
from perspective.core._version import __version__


class TestValidate:
Expand Down Expand Up @@ -53,8 +54,16 @@ def test_validate_filter_is_not_null(self):

def test_validate_expressions(self):
computed = ["// expression1 \n 'Hello'"]
assert validate.validate_expressions(["// expression1 \n 'Hello'"]) == computed
assert validate.validate_expressions(computed) == computed
computed = [{"name": "expression1", "expr": "'hey'"}]
assert validate.validate_expressions(computed) == computed

def test_validate_expressions_invalid(self):
with raises(PerspectiveError):
assert validate.validate_expressions({})

def test_validate_version(self):
assert validate.validate_version("1.2.3")
assert validate.validate_version("0.0.0+1.2.3")
assert not validate.validate_version("abc")
assert validate.validate_version(__version__)
11 changes: 10 additions & 1 deletion python/perspective/perspective/viewer/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,10 @@ def validate_expressions(expressions):

if isinstance(expressions, list):
for expr in expressions:
if not isinstance(expr, str):
if isinstance(expr, dict):
if not (expr.get("name") and expr.get("expr")):
raise PerspectiveError("Cannot parse dict expression: {}".format(str(expr)))
elif not isinstance(expr, str):
raise PerspectiveError("Cannot parse non-string expression: {}".format(str(type(expr))))
return expressions
else:
Expand All @@ -158,3 +161,9 @@ def validate_plugin_config(plugin_config):

def validate_title(title):
return title


def validate_version(version):
# basic semver of form \d+\.\d+\.\d+(\+.+)?
spl = version.split(".", 2)
return len(spl) == 3 and spl[0].isdigit() and spl[1].isdigit() and (spl[2].split("+")[0]).isdigit()
1 change: 1 addition & 0 deletions python/perspective/perspective/viewer/viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class PerspectiveViewer(PerspectiveTraitlets, object):
"theme",
"settings",
"title",
"version",
)

def __init__(
Expand Down
8 changes: 8 additions & 0 deletions python/perspective/perspective/viewer/viewer_traitlets.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
# ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

from traitlets import HasTraits, Unicode, List, Bool, Dict, validate
from ..core._version import __version__

from .validate import (
validate_plugin,
validate_columns,
Expand All @@ -22,6 +24,7 @@
validate_expressions,
validate_plugin_config,
validate_title,
validate_version,
)


Expand Down Expand Up @@ -54,6 +57,7 @@ class PerspectiveTraitlets(HasTraits):
server = Bool(False).tag(sync=True)
client = Bool(False).tag(sync=True)
title = Unicode(None, allow_none=True).tag(sync=True)
version = Unicode(__version__).tag(sync=True)

@validate("plugin")
def _validate_plugin(self, proposal):
Expand Down Expand Up @@ -94,3 +98,7 @@ def _validate_plugin_config(self, proposal):
@validate("title")
def _validate_title(self, proposal):
return validate_title(proposal.value)

@validate("version")
def _validate_version(self, proposal):
return validate_version(proposal.value)
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,12 @@ impl yew::Component for SymbolAttr {
.and_then(|json_val| {
serde_json::from_value::<SymbolConfig>(json_val.clone())
.ok()
.map(|s| s.symbols.into_iter().map(|s| s.into()).collect_vec())
.map(|s| {
s.symbols
.into_iter()
.map(|s| SymbolKVPair::new(Some(s.0), s.1))
.collect_vec()
})
})
.unwrap_or_default();

Expand All @@ -101,7 +106,7 @@ impl yew::Component for SymbolAttr {
let serialized = new_pairs
.clone()
.into_iter()
.filter_map(|pair| pair.try_into().ok())
.filter_map(|pair| Some((pair.key?, pair.value)))
.collect();

p.send_plugin_config(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,15 @@
// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

use std::collections::HashMap;

use serde::{Deserialize, Serialize};

use crate::components::containers::kvpair::KVPair;
use crate::utils::ApiError;

#[derive(Serialize, Deserialize)]
pub struct SymbolConfig {
pub symbols: Vec<SymbolSerde>,
pub symbols: HashMap<String, String>,
}
#[derive(Serialize, Deserialize)]
pub struct SymbolSerde(pub KVPair<String, String>);

pub type SymbolKVPair = KVPair<Option<String>, String>;
impl TryFrom<SymbolKVPair> for SymbolSerde {
type Error = ApiError;

fn try_from(pair: SymbolKVPair) -> Result<Self, Self::Error> {
Ok(SymbolSerde(KVPair {
key: pair
.key
.ok_or::<Self::Error>("Could not unwrap {pair:?}".into())?,
value: pair.value,
}))
}
}
impl From<SymbolSerde> for SymbolKVPair {
fn from(pair: SymbolSerde) -> Self {
Self {
key: Some(pair.0.key),
value: pair.0.value,
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,8 @@ where
value,
}
}

pub fn tuple(&self) -> (&K, &V) {
(&self.key, &self.value)
}
}
Loading
Loading