Skip to content

Simple example transforms#8

Merged
cmungall merged 7 commits intomainfrom
transform_examples
May 27, 2023
Merged

Simple example transforms#8
cmungall merged 7 commits intomainfrom
transform_examples

Conversation

@ialarmedalien
Copy link
Collaborator

@ialarmedalien ialarmedalien commented May 4, 2023

A few examples of some simple transforms that I would like to do, plus a possible implementation using the asteval package.

Forgot to add the asteval package to the toml file so all the tests are failing ATM. Everything also needs to be updated to take into account the new changes from the compiler addition PR.

Comment on lines +1 to +5
"""
meta-circular interpreter for evaluating python expressions

- See `<https://stackoverflow.com/questions/2371436/evaluating-a-mathematical-expression-in-a-string>`_
"""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

copied this file over from linkml_runtime as I needed to hack the existing eval_expr function a bit.

Comment on lines +93 to +95
if expr == "None":
# TODO: do this as part of parsing
return None
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed this from if "None" in expr to make it less permissive

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

...although I think it needs to go completely. Not sure if there are cases in the main codebase where this functionality is required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is the only change to this file.

from dataclasses import dataclass
from typing import Any, Dict, Optional, Type, Union

from asteval import Interpreter
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had a look at various different packages for "safely" evaluating strings as python. This one seemed fairly reasonable in allowing a range of useful operations. Obviously evaluating user input is always going to be dodgy, and it's particularly bad in python. Anyway, I wanted something that would allow me to demonstrate the kinds of transform that I will need to do whilst making it slightly harder for potential hackers who are keenly monitoring this repo to take over everyone's computers.

Comment on lines +120 to +122
aeval = Interpreter(usersyms={"src": ctxt_obj, "target": None})
aeval(slot_derivation.expr)
v = aeval.symtable["target"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This sets up the context object as the variable src and gets the slot value from the variable target. Example transform:

d_test = [x.important_event_date for x in src.has_important_life_events if str(x.event_name) == "PASSED_DRIVING_TEST"]
target = d_test[0] if d_test else None

If you use **ctxt_dict, it is easy to accidentally have a variable with the same name as python function/keyword, hence restricting everything to be under src.

@codecov-commenter
Copy link

codecov-commenter commented May 26, 2023

Codecov Report

Merging #8 (b7319dc) into main (df40ae6) will decrease coverage by 1.77%.
The diff coverage is 74.33%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
- Coverage   86.51%   84.75%   -1.77%     
==========================================
  Files          20       21       +1     
  Lines         749      846      +97     
==========================================
+ Hits          648      717      +69     
- Misses        101      129      +28     
Impacted Files Coverage Δ
src/linkml_transformer/compiler/python_compiler.py 95.65% <ø> (ø)
src/linkml_transformer/transformer/eval_utils.py 71.42% <71.42%> (ø)
...nkml_transformer/transformer/object_transformer.py 91.34% <93.33%> (-0.24%) ⬇️

... and 4 files with indirect coverage changes

Comment on lines +28 to +32
{%- if '\n' in sd.expr -%}
gen_{{ sd.name }}(source_object)
{%- elif '{' in sd.expr and '}' in sd.expr -%}
{{ sd.expr|replace('{', '')|replace('}', '') }}
{%- else -%}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reproduce multi-line expressions and var names with { and } around them

Comment on lines +93 to +95
if expr == "None":
# TODO: do this as part of parsing
return None
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

...although I think it needs to go completely. Not sure if there are cases in the main codebase where this functionality is required.

Comment on lines +175 to +176
source_type = type(source_obj)
source_type_name = source_type.__name__
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

renamed for clarity

@ialarmedalien ialarmedalien marked this pull request as ready for review May 26, 2023 19:59
secondary_email:
expr: "NULL"

# FIXME: this should return the type "None", rather than null
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is dodgy -- not sure what the intention is, whether "None" should be treated as a string by the transformer or whether it should be treated as null (the current MO). IMHO it should be a string as "NULL" already sets the field to null in the transformation.

@cmungall cmungall merged commit 9066675 into main May 27, 2023
@cmungall cmungall deleted the transform_examples branch May 27, 2023 01:42
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.

3 participants