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

Add UnivariateTimeTypeToContinous - take II #295

Merged
merged 21 commits into from
Aug 25, 2020
Merged

Add UnivariateTimeTypeToContinous - take II #295

merged 21 commits into from
Aug 25, 2020

Conversation

ablaom
Copy link
Member

@ablaom ablaom commented Aug 14, 2020

continuation of #245

Carl Morris and others added 20 commits April 27, 2020 19:00
Fix default constructor case of Hour(24) when applied to a Date vector.
Hour cannot be added to Date objects, so the step must be converted to
Day, but because this is a change to the step hyper parameter it must be
adjusted. Unfortunately, this case cannot be distinguished from a user
supplied date, so a warning must be shown, which is non-ideal for a
common default case.

Alternatively we can add an extra internal variable to track whether a
user supplied step is given and only warn on that case, but that is
extra complexity I am avoiding for this commit.
@ablaom
Copy link
Member Author

ablaom commented Aug 14, 2020

The fail is due to package conflict. This request should resolve it:

JuliaNLSolvers/Optim.jl#840 (comment)

@ablaom
Copy link
Member Author

ablaom commented Aug 14, 2020

@venuur In the tests on Julia 1.4 that run okay, there are some warnings being issued for the transformer. Be good if you can confirm they are to be expected and then I will silence them with @test_logs. The log is here:

https://travis-ci.com/github/alan-turing-institute/MLJModels.jl/jobs/371957210

@venuur
Copy link
Contributor

venuur commented Aug 14, 2020 via email

@ablaom
Copy link
Member Author

ablaom commented Aug 25, 2020

Re-opening to retrigger CI

@ablaom ablaom closed this Aug 25, 2020
@ablaom ablaom reopened this Aug 25, 2020
@ablaom
Copy link
Member Author

ablaom commented Aug 25, 2020

We can ignore the branch fail here b/s PR test pass and the fail is due to test pkg conflict which is resolved on the base of PR.

@ablaom ablaom merged commit 743b9ac into dev Aug 25, 2020
@ablaom ablaom mentioned this pull request Sep 1, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants