Kazadhum / file_intro_plugin

0 stars 0 forks source link

Rigelfile Simplification #3

Open Kazadhum opened 1 year ago

Kazadhum commented 1 year ago

The next thing to do here is to simplify the Rigelfile field corresponding to this plugin.

Specifically, we can "transfer" most - if not all - of the other fields to a dictionary for each column. That way, it is possible to do a verification and introspection on multiple axes.

Additionally, it would also be pertinent to add the functionality of checking for the existence - or lack thereof - of certain strings, so the plugin can also analyze non-numerical results.

miguelriemoliveira commented 1 year ago

Sounds good. Let's see what @rarrais says ...

Kazadhum commented 1 year ago

Sounds good. Let's see what @rarrais says ...

This was his suggestion, so I'm sure he agrees :smile:

Kazadhum commented 1 year ago

Started working on this, refactoring all of the code seems like the easiest way to do this. Right now I got the plugin to acquire the data from the Rigelfile. Since everything is in a dictionary now, I've gotten my plugin to check the data types in the fields and issue a warning.

As far as I'm aware, there is no way to specify what each different field in a dictionary is supposed to be, so I'm using the 'Any' type in the pydantic model:

introspection_target_columns: List[Dict[str, Dict[str, Any]]]

Maybe there is a way to do this and I just didn't find it, though. Do you know if this is possible, @MisterOwlPT?

The next step is adding some conditions for the plugin to work, since some fields can be contradictory. For example, if use_latest_row == True and value_row != None, how is the plugin supposed to know which value to select for comparison? So I'm adding some conditions and warnings to address this. After that, I'm going to write the code for the verification step, as well as the introspection part.

By the way, in this iteration of the plugin, I'm doing all of this for every column, so we can perform validation and introspection for various columns.

rarrais commented 1 year ago

The next step is adding some conditions for the plugin to work, since some fields can be contradictory. For example, if use_latest_row == True and value_row != None, how is the plugin supposed to know which value to select for comparison? So I'm adding some conditions and warnings to address this. After that, I'm going to write the code for the verification step, as well as the introspection part.

Warning the user about the conditions that are mutually exclusive sounds like a good idea.

MisterOwlPT commented 1 year ago

Hi @Kazadhum,

Sorry once again for the delay in the answer.

As far as I'm aware, there is no way to specify what each different field in a dictionary is supposed to be, so I'm using the 'Any' type in the pydantic model:

You are correct in this case, due to the structure of your Pydantic model.

Please consider the hypothetical YAML excerpt:

# ...
food: [
  yogurt:
    brand: "Yummy"
    quantity: 6
  carrot:
    brand: "Maggot's Farm"
    quantity: 10
]
# ...

You could use the following Pydantic model to parse/validate the said example as such:

from pydantic import BaseModel

class Groceries(BaseModel):
  # ...
  food: List[Dict[str, Dict[str, Any]]]
  # ...

I believe this example is somewhat similar to what you have right?

If so, there's a better way to do this! Consider the following Python code for the same example:

from pydantic import BaseModel

class FoodItem(BaseModel):
  brand: str
  quantity: int

class Groceries(BaseModel):
  # ...
  food: List[Dict[str, FoodItem]]  # <-- notice the difference here
  # ...

Using Pydantic recursive models you can add more details to the model and also increase its readability while keeping the same structure - the type List[Dict[str, FoodItem]] is more comprehensible than List[Dict[str, Dict[str, Any]]].

Rule of thumb: Any should only be used when you don't have an alternative (for instance when working with untyped libraries) 😃

I hope you find this message helpful. Let me know if you have more questions or doubts.

Kazadhum commented 1 year ago

Thank you @MisterOwlPT! This is exactly what I was looking for, but I wasn't sure it was possible. I'll get on this right away!