pydantic / speedate

Fast and simple datetime, date, time and duration parsing for rust.
https://docs.rs/speedate/latest/speedate/
MIT License
199 stars 17 forks source link

Add `unix_timestamp_offset` to `TimeConfig` #45

Closed adriangb closed 1 year ago

adriangb commented 1 year ago

Fix #41.

adriangb commented 1 year ago

please review

codecov[bot] commented 1 year ago

Codecov Report

Merging #45 (96d31a1) into main (ed75c5b) will decrease coverage by 0.11%. The diff coverage is 93.93%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #45 +/- ## ========================================== - Coverage 99.17% 99.07% -0.11% ========================================== Files 6 6 Lines 847 862 +15 ========================================== + Hits 840 854 +14 - Misses 7 8 +1 ``` | [Impacted Files](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic) | Coverage Δ | | |---|---|---| | [src/lib.rs](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic#diff-c3JjL2xpYi5ycw==) | `91.66% <0.00%> (-8.34%)` | :arrow_down: | | [src/time.rs](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic#diff-c3JjL3RpbWUucnM=) | `97.51% <92.30%> (+0.07%)` | :arrow_up: | | [src/datetime.rs](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic#diff-c3JjL2RhdGV0aW1lLnJz) | `100.00% <100.00%> (ø)` | | | [src/duration.rs](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic#diff-c3JjL2R1cmF0aW9uLnJz) | `100.00% <100.00%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic). Last update [ed75c5b...96d31a1](https://app.codecov.io/gh/pydantic/speedate/pull/45?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydantic).
samuelcolvin commented 1 year ago

I think this should be unix_tz a bool or enum with two options.

adriangb commented 1 year ago

or enum with two options

It is an enum with two options

dmontagu commented 1 year ago

I think this should be unix_tz a bool or enum with two options.

Do you mean the name of the field on TimeConfig? Or something else?

#[derive(Debug, Clone, Default, Copy)]
pub enum DefaultTimeOffset {
    #[default]
    Naive,
    Utc,
}

// ...

#[derive(Debug, Clone, Default)]
pub struct TimeConfig {
    pub microseconds_precision_overflow_behavior: MicrosecondsPrecisionOverflowBehavior,
    pub default_time_offset: DefaultTimeOffset,
}

Are you just suggesting changing default_time_offset to unix_tz?

samuelcolvin commented 1 year ago

If the input is a number or a numeric string both of which are interpreted as Unix timestamps, then this setting should decide if the timezone is None or Some(0).

Hence the struct member name should change to unix_tz.

adriangb commented 1 year ago

Is this what you mean? https://github.com/pydantic/speedate/pull/45/commits/d39415f2d0efc9c3be31b884c0e4173808b961ef#diff-cc856031f5a2bd26067d2ed100ee6e0dc576d5ed7c354a6f223468b7e5b85664

adriangb commented 1 year ago

please review