search

JuliaPy / PyCall.jl

Package to call Python functions from the Julia language
MIT License
1.47k stars 187 forks source link

Add parsing for datetime.time #1069

Closed nateybear closed 6 months ago

nateybear commented 9 months ago

Using same patterns as elsewhere in this file (and avoiding timezones like in DateTime), add support for parsing times from the Python datetime package to Julia's Dates.Time struct. This came up as a need I had in a personal project and was a quick addition.

stevengj commented 9 months ago

Looks good. Could you add a couple of tests?

codecov-commenter commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (d407513) 67.75% compared to head (c74af34) 68.02%. Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1069 +/- ## ========================================== + Coverage 67.75% 68.02% +0.27% ========================================== Files 20 20 Lines 2025 2036 +11 ========================================== + Hits 1372 1385 +13 + Misses 653 651 -2 ``` | [Flag](https://app.codecov.io/gh/JuliaPy/PyCall.jl/pull/1069/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaPy) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/JuliaPy/PyCall.jl/pull/1069/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaPy) | `68.02% <100.00%> (+0.27%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaPy#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nateybear commented 9 months ago

Looks good. Could you add a couple of tests?

I added a rountripeq test like for Date and DateTime

ETA: added @test_throws to satisfy coverage checker too

nateybear commented 6 months ago

@stevengj bumping since I also forgot about this. Tests have been implemented, check failures seem unrelated to this change.

stevengj commented 6 months ago

Looks good, thanks!