Types: Improve support for FLOAT type, converging to FLOAT vs. DOUBLE#265
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
amotl
force-pushed
the
types-improve-float
branch
from
b2bf840 to
2a96fad
Compare
kneth
left a comment
kneth
left a comment
There was a problem hiding this comment.
Are Nan, Inf and -Inf also supported? If so, should it be tested?
|
Hm. Maybe good idea, maybe not applicable here. No idea. If you agree with the change, because it solves a relevant obstacle in supertask and possibly others, please merge at your disposal. We can also ask @matriv to have a second look. 🙏 |
| float_double_data = { | ||
| "col_1": [19556.88, 629414.27, 51570.0, 2933.52, 20338.98], | ||
| "col_2": [ | ||
| 15379.920000000002, | ||
| 1107140.42, | ||
| 8081.999999999999, | ||
| 1570.0300000000002, | ||
| 29468.539999999997, | ||
| ], | ||
| } | ||
| float_double_df = pd.DataFrame.from_dict(float_double_data) | ||
|
|
There was a problem hiding this comment.
Are
Nan,Infand-Infalso supported? If so, should it be tested?
@kneth suggested to test for special values. Maybe relevant tests exist already, otherwise let's do it?
There was a problem hiding this comment.
I've added a new one to test "Nan, Inf and -Inf" values, all stored as NULL in CrateDB so they all read back as NaN in pandas.
| "cratedb-toolkit[testing]", | ||
| "dask[dataframe]", | ||
| "pandas<2.4", | ||
| "pandas[test]<2.4", |
There was a problem hiding this comment.
Let's also validate with pandas 3.x from the beginning, i.e. use pandas[test]<3.1 here?
There was a problem hiding this comment.
I think Separate PR is better, pandas upgrade is not in the scope of the PR.
3 participants
About
Improvements to be mainlined from https://github.com/pyveci/supertask/blob/19316e2/supertask/store/cratedb.py.