feature-engine
Add new class to substract datetime variables
This code needs unit tests, but I wanted to get feedback on this newer implementation of the class.
closes stale PR #359
@solegalli This code needs unit tests, but I wanted to get feedback on this newer implementation of the class to see if it aligns with your expectations.
It looks like the failures are happening due to an unrelated boxcox failure
No problem! I hope you had a great vacation!
Yes. I fixed that already in the main branch. If you could sync your main branch and then rebase main onto your feature branch, that would solve the issues.
Hi @kylegilde
If I remember correctly, this PR was almost good to go, right? It just needed some tidying of the files in the PR.
Would you have time to try and make those changes?
thanks a lot!
I need to tidy the files and commits, and I need to create real unit tests. I will have time this weekend I believe.
@solegalli I think that I removed the unrelated files. I just need to find time to create unit tests.
Thank you @kylegilde
Looking much better :)
We made a few big changes to the main branch, that is probably why all tests are failing here.
It would be great if you could sync your repo with the main repo, and then rebase main onto this branch to catch up with the latest version of the code:
https://feature-engine.readthedocs.io/en/latest/contribute/contribute_code.html#keep-your-fork-up-to-date
It should not affect your code too much, apart from the imports at the top.
@solegalli , I followed your instructions, and the checks ran again, but they failed for the same reasons, which are unrelated to my code changes. What should I try next?
I created a PR to your branch with the changes needed: https://github.com/kylegilde/feature_engine/pull/2
And I noticed that you did not rebase main onto that branch. That is why the tests are failing.
- You need to checkout your main branch (
git checkout main) - Pull the last version of feature engine main into your local main (
git pull upstream main) - then checkout your feature branch (
git checkout dt_subtraction) - and once there do:
git rebase main
That should work. After that, you should see the few changes that I made in the PR.
If you want to ignore my PR, the only need you need to fix in your new class are the imports at the top of your new class.
I merged your PR. I see your commit in the commit history, but we are getting errors.
I tried following your Docs instructions one more time, but it did not work.
I'm sorry! I'm very new to forking.
Maybe it is because yesterday we did another merge with some structural changes?
Could you try syncing your fork again and rebasing main to your feature branch? then maybe force push your feature branch?
So after you repeat (again) the 4 bullets in my previous message try:
git push -f origin dt_subtraction
You'll know that it worked when here you don't see irrelevant files like the DropHighPSI features
How are you coming along?
Maybe you'd like to sync your main branch, then create a new feature branch from there, copy and paste the 3 new files from this PR into the new branch and make new PR? Maybe that is easier?
I'm heading out on vacation on Thursday. I would love to, but I don't know if I will get a chance to try this before then.
On Tue, Aug 30, 2022 at 4:06 AM Soledad Galli @.***> wrote:
How are you coming along?
Maybe you'd like to sync your main branch, then create a new feature branch from there, copy and paste the 3 new files from this PR into the new branch and make new PR? Maybe that is easier?
— Reply to this email directly, view it on GitHub https://github.com/feature-engine/feature_engine/pull/486#issuecomment-1231382797, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXCO724ZCUCEYHHKOK56Q3V3XFK5ANCNFSM53LIJGOQ . You are receiving this because you were mentioned.Message ID: @.***>
-- Kyle Gilde