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

Complete support for DateTime and Duration coordinate #36

Closed
serzhiio opened this issue Sep 25, 2019 · 13 comments
Closed

Complete support for DateTime and Duration coordinate #36

serzhiio opened this issue Sep 25, 2019 · 13 comments
Labels
bug Something isn't working feature request

Comments

@serzhiio
Copy link
Contributor

Is it possible to build chart with seconds or even milliseconds on X-axis? It would be nice to split it by some nice ranges like 5sec or 1/5min?

@serzhiio
Copy link
Contributor Author

It looks like Datetime require only Date not DateTime?
pub struct RangedDate<Z: TimeZone>(Date<Z>, Date<Z>);

@38 38 changed the title Seconds/Milliseconds ranged chart? Complete support for DateTime and Duration coordinate Sep 25, 2019
@38 38 added bug Something isn't working feature request labels Sep 25, 2019
@38
Copy link
Member

38 commented Sep 25, 2019

That's something I have no chance to implement yet. But I am going to clean this up very soon.
Basically, both DateTime and Duration should be supported.

@serzhiio
Copy link
Contributor Author

In datetime mod there is RangedDateTime struct which is not developed.
Is it really needed to have two Date and DateTime coords structs? Why not implement
IntoMonthly, IntoYearly, Into Hours, Into Minutes, IntoSeconds for one DateTime struct?

@serzhiio
Copy link
Contributor Author

And use chrono::DateTime instead of chrono::Date.

@38
Copy link
Member

38 commented Sep 30, 2019

And use chrono::DateTime instead of chrono::Date.

That is mostly right. But it will cause problem if the DateTime range is smaller than one day.

But we can add additional logic to the existing date handling code make it also produce, hourly, ...., etc.

Why not implement IntoMonthly, IntoYearly, Into Hours, Into Minutes, IntoSeconds for one DateTime struct?

And I would like to mention the reason why IntoMonthly and IntoYearly exists is because month and year aren't equally length. Thus, there's many weird things may happen when we want to have monthly or yearly granularity. Basically, you can not get real monthly or yearly granularity by simply splitting the range. And this is what those trait for, the reason for there's no IntoWeekly and IntoDaily is the same, because they always have the same length, and all those need should be fulfilled by tweaking the number of labels.

It seems some amount of work to do to support all those things. I will think about a concrete plan to support this. And I probably have time to address this in a day or two.

Please let me know if you have any other concerns.

@38
Copy link
Member

38 commented Sep 30, 2019

@serzhiio Just a quick update, monthly and yearly coordinate should be able to accept DateTime now.

I've just refactored a little bit but still not tested it. And the next step should be add code that identify the granularity for a DateTime range. And if we need to use a granularity which is bigger than a day, we can simply reuse the code for Date.

I will let you know when this happen

@serzhiio
Copy link
Contributor Author

This sounds sweet. I'd like to help, but your code a bit complicated for me (too many traits :))
I'm exploring ability to work with milliseconds, this is all i need now.

@serzhiio
Copy link
Contributor Author

error[E0277]: the trait bound `std::ops::Range<chrono::datetime::DateTime<chrono::offset::utc::Utc>>: plotters::coord::ranged::Ranged` is not satisfied
   --> src\scenes\instances\components\chart\mod.rs:173:14
    |
173 |             .build_ranged(from..to, min..max).unwrap();
    |              ^^^^^^^^^^^^ the trait `plotters::coord::ranged::Ranged` is not implemented for `std::ops::Range<chrono::datetime::DateTime<chrono::offset::utc::Utc>>`

@38
Copy link
Member

38 commented Sep 30, 2019

No, I mean it support :(a..b).monthly() , (a..b).yearly()

But now it supports datetime completely.

Finally get this dome ;)

Closing the issue, feel free to reopen it if you find anything is wrong.

@38 38 closed this as completed Sep 30, 2019
@serzhiio
Copy link
Contributor Author

Hmm.. it looks like working, but there are no labels at least on 3 minutes timespan?

@38
Copy link
Member

38 commented Sep 30, 2019

image

You mean this, it works on my side. Would you like to send a minimal code reproduce what you have seen?

@serzhiio
Copy link
Contributor Author

serzhiio commented Oct 1, 2019

Its working as supposed after latest bugfix.
Is it possible to format label to show only minutes+sec like ":30:15"?

@serzhiio
Copy link
Contributor Author

serzhiio commented Oct 1, 2019

Ive managed this via .x_lable_formatter().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature request
Projects
None yet
Development

No branches or pull requests

2 participants