Closed abstractqqq closed 1 month ago
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
functime-docs | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | May 20, 2024 7:07am |
Ciao tq, thank you very much for the PR. Will try to review this ASAP. Not super expert on this branch of the codebase.
Ciao tq, thank you very much for the PR. Will try to review this ASAP. Not super expert on this branch of the codebase.
In my second commit, I removed a duplicate test in ts_fresh and fixed a bug in yeojohnson in test_preprocess
Passed all tears. Tears of joy 🥹
OK here I am. Just two things before approving:
Then we're ready to go. Thank you very much.
I explained in my commit's comment, here is short version: the "nan_case" tests are unnecessary in my opinion. This occurs when some feature is called on a null series ( completely empty). In that case let Polars throw the user an error. If we really want to handle that case, we will need to write a pl.when(input.len() == 0).then().otherwise()... which in my opinion is quite pointless and redundant
Ciao tq, sorry for getting back late. One last question: I saw you added the rust-toolchain file. Should we keep it pinned with 2024-04-15
? If so, I will just run a round of linter + formatter and merge :)
Adding the toolchain file helped me solve some issues regarding package updates (polars rs and faer)
My local run didn't cause any problems so I guess it will be fine. Let it spin and take a look?
Adding the toolchain file helped me solve some issues regarding package updates (polars rs and faer)
My local run didn't cause any problems so I guess it will be fine. Let it spin and take a look?
Yes, sorry if I wasn't clear. I wonder why you used nightly-2024-04-15
but it makes sense. It's rust 1.79 but "frozen" at that date. Will merge PR. We can switch to stable when it's released (I read June 13th).
Reference Issues/PRs
What does this implement/fix? Explain your changes.
Now all tests in test_tsfresh.py should pass.
Any other comments?