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

Converting Dates.FixedPeriod to Quantity{...,𝐓} #329

Closed
wants to merge 5 commits into from

Conversation

mcabbott
Copy link
Contributor

@mcabbott mcabbott commented May 12, 2020

Fixes #125, or at least makes a start.

Edit: I moved this to a new file, as it needs to come after the relevant units have been defined.

julia> uconvert(s, Microsecond(1))
1//1000000 s

julia> convert(typeof(1f0s), Minute(1))
60.0f0 s

julia> promote(Minute(1), 1s) 
(60 s, 1 s)

Should things like this be made to work?

julia> Minute(1) + 1s
ERROR: MethodError: no method matching +(::Minute, ::Quantity{Int64,𝐓,Unitful.FreeUnits{(s,)

@sostock
Copy link
Collaborator

sostock commented May 13, 2020

I also started to implement this some time ago, I have uploaded it here so you can maybe use some of my code:

https://gist.github.com/sostock/68e5eda1abee911116dcc08970843d0c

The broken tests should work on Julia ≥ 1.5, since JuliaLang/julia#34645 was fixed recently.

src/dates.jl Outdated Show resolved Hide resolved
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
@mcabbott
Copy link
Contributor Author

OK, perhaps I should close this in favour of that, you've taken this far more seriously.

Would the plan be to just not run those tests on Julia < 1.5?

@codecov-io
Copy link

Codecov Report

Merging #329 into master will decrease coverage by 0.25%.
The diff coverage is 65.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #329      +/-   ##
==========================================
- Coverage   80.23%   79.98%   -0.26%     
==========================================
  Files          15       16       +1     
  Lines        1174     1194      +20     
==========================================
+ Hits          942      955      +13     
- Misses        232      239       +7     
Impacted Files Coverage Δ
src/Unitful.jl 100.00% <ø> (ø)
src/dates.jl 65.00% <65.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e97233b...35bb6ce. Read the comment docs.

@sostock
Copy link
Collaborator

sostock commented May 14, 2020

OK, perhaps I should close this in favour of that, you've taken this far more seriously.

If you want, I can make a PR with my changes. I think I didn’t do that because I wanted to implement some more stuff (like support for CompoundPeriods) before making a PR.

Would the plan be to just not run those tests on Julia < 1.5?

Yes, that’s how I would do it.

@mcabbott
Copy link
Contributor Author

OK, no great rush, but having even the basics would be nice.

@mcabbott mcabbott closed this May 14, 2020
@sostock
Copy link
Collaborator

sostock commented May 14, 2020

I opened #331 as a replacement.

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.

Support TimeTypes
3 participants