bug: FloatMetadataProperty: value is not a valid float when it is an rounded integer

[sdiazlor]

Description

With the removal of this line, x.0 values will not be rounded so that it passes the FloatMetadataProperty validation. The error comes from the initial definition of the metadata property as float, but then casting and adding as value an integer.

NOTE: This fixes the bug error directly, although this question is still pending @alvarobartt:

However, from a usability perspective there is also the issue of not allowing for passing a rounded float like 1.0 to an IntegerMetadataProperty and perhaps also not an integer like1 to a FloatMetadataProperty.

Closes #4570

Type of change

(Please delete options that are not relevant. Remember to title the PR according to the type of change)

• Bug fix (non-breaking change which fixes an issue)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested

(Please describe the tests that you ran to verify your changes. And ideally, reference tests)

• Test A
• Test B

Checklist

• I followed the style guidelines of this project
• I did a self-review of my code
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I filled out the contributor form (see text above)
• I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)

[alvarobartt]

Hi here @sdiazlor, thanks for the PR! w.r.t. @davidberenstein1957's question, I'm not sure, because if we have an IntegerMetadataProperty we should enforce the values to be int, not casting those to int i.e. 1.0 -> 1; same thing for the FloatMetadataProperty, which shouldn't cast to float by default. So IMO we can keep the casting within the text-descriptives integration while keeping argilla as is, also what do @jfcalvo and @frascuchon think about that?

[github-actions]

The URL of the deployed environment for this PR is https://argilla-quickstart-pr-4605-ki24f765kq-no.a.run.app

[davidberenstein1957]

@sdiazlor could you also check if this work with the error Dani was experiencing?

[davidberenstein1957]

Hi here @sdiazlor, thanks for the PR! w.r.t. @davidberenstein1957's question, I'm not sure, because if we have an IntegerMetadataProperty we should enforce the values to be int, not casting those to int i.e. 1.0 -> 1; same thing for the FloatMetadataProperty, which shouldn't cast to float by default. So IMO we can keep the casting within the text-descriptives integration while keeping argilla as is, also what do @jfcalvo and @frascuchon think about that?

The proposed changed avoids the casting to int right? For me, it was more of a philosophical question about whether or not it made sense to allow for this, outside of this basic PR. I understand why but from a user experience it might be weird that it is impossible to add an 1.0 to an IntegerMetadataProperty or a 1 to a FloatMetadataProperty.

[sdiazlor]

@sdiazlor could you also check if this work with the error Dani was experiencing?

@davidberenstein1957 Yes, I already did. And with a similar case (using to_argilla method), it didn't raise the validation issue. Below, you can see that the values are floats.