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

Layer Data - Add attribute type parser based on value #159

Open
jomey opened this issue Oct 17, 2024 · 5 comments
Open

Layer Data - Add attribute type parser based on value #159

jomey opened this issue Oct 17, 2024 · 5 comments
Labels
DB Any requests related to changes/updates of the DB table structure or definitions enhancement New feature or request

Comments

@jomey
Copy link
Member

jomey commented Oct 17, 2024

An idea from a PR review comment

Since layer data can have multiple data types (i.e. INT for depth, but CHAR for hardness), we could think about adding a feature to the LayerData class that checks the value against known CHAR types and tries to typecast them.

Could also maybe use an ENUM to do this.

@jomey jomey added enhancement New feature or request DB Any requests related to changes/updates of the DB table structure or definitions labels Oct 17, 2024
@aaarendt
Copy link
Contributor

I'm running into this issue right now in #170 as I try to build tests of kwargs value_greater_equal for Layer Data.

It seems there are backend ways to address this, e.g. here.

In the ORM would it work to create Bundles that return custom types based on our measurement type?

@jomey
Copy link
Member Author

jomey commented Nov 25, 2024

There are two ways I can think of how to tackle this:

  • Start a mapping of data types for the value column on the class level and cast them once the user access the column. This will involve overwriting the attribute on the LayerData class.
  • Start a mapping column on the table or with the measurement type that is used to cast the data type. This would involve populating that column when we add new MeasurementTypes. For Example an entries:
    Name Units Derived DataType
    Density kg/m3 False Float
    Hardness False String

@micah-prime
Copy link
Contributor

I'm running into this issue right now in #170 as I try to build tests of kwargs value_greater_equal for Layer Data.

It seems there are backend ways to address this, e.g. here.

In the ORM would it work to create Bundles that return custom types based on our measurement type?

Do we know what caused this functionality to break?
FYI @micahjohnson150 if you're not seeing this

@aaarendt
Copy link
Contributor

Hey @micah-prime , this functionality broke because my new density tests were the first time we tried sending a value into the API logic that tests for greater or less than. Since value is a string it threw an error.

Prior to that we had tested it with dates which had already been cast to the correct format.

For now my proposed fix will be to add an additional type check in the API:

 if "_greater_equal" in k:
      key = k.split("_greater_equal")[0]
      if key == "value":
             qry = qry.filter(
                      cast(getattr(qry_model, key), Numeric) >= v
             )
      else:
            qry = qry.filter(
                      getattr(qry_model, key) >= v
            )

And similarly for _less_equal.

@micah-prime
Copy link
Contributor

Ah makes sense - nice find!

aaarendt added a commit that referenced this issue Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB Any requests related to changes/updates of the DB table structure or definitions enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants