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

xy data loader #1287

Merged
merged 15 commits into from
Sep 28, 2023
Merged

xy data loader #1287

merged 15 commits into from
Sep 28, 2023

Conversation

aarongundel
Copy link
Contributor

@aarongundel aarongundel commented Sep 5, 2023

Adds features relevant for #1218 . Allows for dynamically formatted files to be read. Deprecates other afs formatters (raman reader, xrf reader, fors reader) which are retained for backward compatibility. Adds the concept of an "importer" - an attribute stored on the file tile that points to a database record that specifies a configuration that gets passed to the new xy reader. That configuration tells the XY reader how to parse the file. The reader itself uses the XY Parser utility to do the dirty work of parsing the file. Supports the concept of transformations - provided multiple Y values, transforming those Y values via known methods and functions. As part of this PR, the "mean" or "average" transformation is included (as well as a "basic" transformation which selects the first y value if multiples are given.

Resolves #1218

I suspect we will need to include some kind of import functionality in Arches for Science to transfer these importers between installations and perhaps provide a well-known set, but that is outside the scope of this PR.

@aarongundel aarongundel marked this pull request as ready for review September 5, 2023 21:41
@aarongundel
Copy link
Contributor Author

This PR can be squashed.

@aarongundel aarongundel linked an issue Sep 14, 2023 that may be closed by this pull request
@aarongundel
Copy link
Contributor Author

This PR requires archesproject/arches#10003

@aarongundel
Copy link
Contributor Author

The previously mentioned PR needs to merged into dev/7.5.x for this to work.

Copy link
Member

@chiatt chiatt left a comment

Choose a reason for hiding this comment

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

This looks great. Added just a couple of comments, but I think I'll write them up as other issues.

@@ -139,11 +138,10 @@ def post(self, request):
file_data["renderer"] = next(
(renderer["id"] for renderer in settings.RENDERERS if renderer["name"] == "pdfreader"), None
)
elif dataset_default_format is not None: # instrument was given by zip file name
elif file_data["name"].split(".")[-1] == "dx" or file_data["name"].split(".")[-1] == 'txt': # instrument was given by zip file name
Copy link
Member

Choose a reason for hiding this comment

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

It seems like afs should have a setting with a list of possible 'txt' extensions.

}
const delimiterCharacter = config?.delimiterCharacter ?? ',';

const valueRegex = (delimiterCharacter.length < 2) ? new RegExp(`[${delimiterCharacter}\\s]+`) : new RegExp(`${delimiterCharacter}`);
Copy link
Member

Choose a reason for hiding this comment

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

This regex silently failed when I used ** as a delimiter. This is a pretty unlikely delimiter, but it would be nice to have a message on the front end stating that the delimiter cannot be parsed - just in case.

@chiatt chiatt merged commit 2272a84 into dev/1.1.x Sep 28, 2023
1 check passed
@chiatt chiatt deleted the 1218_2d_data_loader branch September 28, 2023 23:57
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.

Upload step on Upload XY Data provides no feedback on failure Two-Dimensional (Spectral) Digital Data Loader
2 participants