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

transform_value_for_tile for resource-instance datatype simplification/enhancement #11597

Open
whatisgalen opened this issue Oct 31, 2024 · 6 comments
Assignees

Comments

@whatisgalen
Copy link
Member

The ResourceInstanceDataType class supports this kind of value for importing from, for example, csv:

def transform_value_for_tile(self, value, **kwargs):
        try:
            return json.loads(value)
        except ValueError:
            # do this if json (invalid) is formatted with single quotes, re #6390
            try:
                return ast.literal_eval(value)
            except:
                return value
        except TypeError:
            # data should come in as json but python list is accepted as well
            if isinstance(value, list):
                return value

Meaning that each value has to look like this:

{
    "resourceName": "",
    "ontologyProperty": "",
    "inverseOntologyProperty": "",
    "resourceId": ""
}

This datatype already considers data valid if it only contains a value for "resoruceId" and nothing else. It would be nice if a user could import a value that only contained a resourceId uuid string and then the datatype method populates the rest of the boilerplate with empty string values.

@jacobtylerwalls
Copy link
Member

Funny, I just hacked out a version of this in #11596 (not that it's factored perfectly), and I'd definitely be up to pull it out into its own PR or collab on your implementation. Say the word.

@whatisgalen
Copy link
Member Author

I'm hacking out a proof of concept right now to import either a single uuid string or a list of uuid strings from a csv cell. I'd be happy to push to a branch to see if there's a collab that makes sense

@jacobtylerwalls
Copy link
Member

My hack also took a list of python UUID types (not strings), but doesn't handle a jumbled array of some-of-one-and-some-of-the-other, which would be easy to handle if factored well. Happy to review!

@chiatt
Copy link
Member

chiatt commented Nov 11, 2024

The ResourceInstanceDataType class supports this kind of value for importing from, for example, csv:

def transform_value_for_tile(self, value, **kwargs):
        try:
            return json.loads(value)
        except ValueError:
            # do this if json (invalid) is formatted with single quotes, re #6390
            try:
                return ast.literal_eval(value)
            except:
                return value
        except TypeError:
            # data should come in as json but python list is accepted as well
            if isinstance(value, list):
                return value

Meaning that each value has to look like this:

{
    "resourceName": "",
    "ontologyProperty": "",
    "inverseOntologyProperty": "",
    "resourceId": ""
}

This datatype already considers data valid if it only contains a value for "resoruceId" and nothing else. It would be nice if a user could import a value that only contained a resourceId uuid string and then the datatype method populates the rest of the boilerplate with empty string values.

It would be important to use the default values for the relationship and inverse relationship if they are available rather than empty strings.

@jacobtylerwalls
Copy link
Member

Great point, and that makes this trickier when multiple graphs are allowed. To get the correct ontology props, we'd need the graph, which we don't have from an incoming UUID. We do query the resource table with the incoming UUID in validate(strict=True), so perhaps we should have some temporary lookup state like the concept datatypes do to avoid multiple queries.

@jacobtylerwalls jacobtylerwalls moved this to 🏗 In Progress in pipeline Nov 12, 2024
@whatisgalen
Copy link
Member Author

whatisgalen commented Nov 20, 2024

Don't hate me but it would be really great to also include a single value or comma-separated list of "legacyid" strings. I think it's important to have some kind of proxy for a related resource much like how we have a legacyid as a proxy for the resourceinstance elsewhere in Arches.

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

No branches or pull requests

3 participants