[FIX] Edit Domain: Improve Text/Categorical to Time conversion #4601
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4601 +/- ##
==========================================
+ Coverage 83.55% 83.62% +0.07%
==========================================
Files 281 276 -5
Lines 56104 55462 -642
==========================================
- Hits 46877 46382 -495
+ Misses 9227 9080 -147 |
| @@ -917,6 +918,19 @@ def test_column_str_repr(self): | |||
| d = column_str_repr(v, np.array([0., np.nan, 1.0])) | |||
| assert_array_equal(d, ["00:00:00", "?", "00:00:01"]) | |||
|
|
|||
| def test_time_parse(self): | |||
| """parsing additional datetimes by pandas""" | |||
| date = ["1/22/20", "1/23/20", "1/24/20"] | |||
There was a problem hiding this comment.
How about when dates are 1/1/20? How do you know, which is day and which is month? M/D/Y is not universal.
There was a problem hiding this comment.
If data is not in iso8601 format then month first is assumed. Trying to parse dates without specifying the format is always tricky. Perhaps an additional field to specify d-m-y order would be the solution (to_datetime has dayfirst and yearfirst parameters). However, that would probably be in a new pr. I would really like to merge this since I need it to parse such dates.
ajdapretnar
changed the title
Orange/widgets/data/oweditdomain.py
Outdated
| dti = pd.to_datetime(values, errors="coerce") | ||
| _values = str_to_time(np.array(values)) |
There was a problem hiding this comment.
You extracted some code into a new function str_to_time to reuse it here, but then have to call pd.to_datetime outside of the new function anyway, to get some extra info...
robertcv
force-pushed
the
enh/editdomain_time
branch
from
6229dbf
to
fcf24c6
Compare
lanzagar
changed the title
Issue
New TimeVariable is initialized considering its parsing of string but actual data is converted by pandas.
This results in two issues:
Description of changes
Move pandas transformation into a separate function and apply it accordingly.
Includes