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

n-dimensional support via parse_index #41

Merged
merged 17 commits into from
Nov 20, 2020
Merged

Conversation

telamonian
Copy link
Member

@telamonian telamonian commented Oct 20, 2020

Fixes #4, #18, #22, #37

Adds support for datasets with ndim >2 (issue #4) as well as datasets with ndim<=1 (issue #18). The slice input box will now accept the complete subset of numpy indices that are some combination of integer indexes and slices. Though for practical purposes of display, the number of slices in the supplied index has to be between 0-2.

The PR is now mostly done. Still needs a couple of things:

  • 0D support, for the case of a single scalar value returned by an index without slices
  • 0D support, for the case of data with an empty shape
  • 1D dataset support
  • doesn't break support for 2D datasets
  • full n-dimensional dataset support, for n=3 all the way up to n=32 (the default max the HDF5 lib is compiled with)
  • Some kind of UI element that displays the original shape of the currently open dataset
  • Possibly also a UI element that displays the current visible shape of the currently open dataset
  • Better error handling/validation w.r.t. user input of the index string

This PR should also fix the bugs reported in #22 and #37, due to the complete overhaul of how data and metadata is handled, and also thanks to @JonjonHays handy json fix.

@telamonian
Copy link
Member Author

The PR now adds full support for all 0D, 1D, 2D, and beyond cases. I've tested it, and I think this is ready to go in.

There is still some unfinished business listed on the checklist in this PR's first post. These issues are all a bit out of the scope of enabling basic n-dimensional support, so I'll address these remaining issues in future PRs. As a note to myself, when the code here raises an error, in addition to doing what it's supposed to it also raises an extra "uncaught in promise error":

jupyterlab-hdf5/src/hdf.ts

Lines 121 to 152 in 0531f3f

export function hdfApiRequest(
url: string,
body: JSONObject,
settings: ServerConnection.ISettings
): Promise<any> {
return ServerConnection.makeRequest(url, body, settings).then(response => {
if (response.status !== 200) {
return response.text().then(data => {
let json;
if (data.length > 0) {
try {
// HTTPError on the python side adds some leading cruft, strip it
json = JSON.parse(data.substring(data.indexOf("{")));
} catch (error) {}
}
if (json?.type === "JhdfError") {
const { message, debugVars, traceback } = json;
throw new HdfResponseError({
response,
message,
debugVars,
traceback
});
} else {
throw new ServerConnection.ResponseError(response, data);
}
});
}
return response.json();
});
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for n-dimensional datasets
2 participants