[ENH] Feature Constructor: Easier categorical features, allow creation of date/time, easier use of string data #3936

janezd · 2019-07-11T20:57:41Z

Fixes #2900. Fixes #3056.

Lets the expression for discrete variables return strings instead of indices.

If the user defines values, the function must return integer indices. If function returns strings (or instances of Value), the user mustn't define values. It is contrived, but you can't have both. We will have to mention this in documentation. :) If you have a better solution, feel free to change the code.
Casts string values into strings, so name[0] is now a valid expression for zoo. Before, you'd have to write str(name)[0] -- and, of course, list all possible values.
Allows creation of Date / Time variables. If expression returns a string, it is parsed as date/time.

Includes

Code changes
Tests
Documentation

codecov · 2019-07-11T21:07:57Z

Codecov Report

Merging #3936 into master will decrease coverage by 0.24%.
The diff coverage is 69.38%.

@@            Coverage Diff            @@
##           master   #3936      +/-   ##
=========================================
- Coverage   84.95%   84.7%   -0.25%     
=========================================
  Files         376     371       -5     
  Lines       65907   64957     -950     
=========================================
- Hits        55988   55022     -966     
- Misses       9919    9935      +16

ajdapretnar · 2019-07-12T10:25:55Z

The idea is the right one, but it doesn't seem to work. When I create a datetime variable from string, which returns '2019-01-01' for example, the widget says ValueError: Could not convert string to float: '2019-01-01'. There is a special way of creating datetime variables that has to be considered here.

janezd · 2019-07-12T10:43:18Z

When I create a datetime variable from string, which returns '2019-01-01' for example, the widget says ValueError: Could not convert string to float: '2019-01-01'.

That's why I asked whether it's useful - because I suspected it's not. :) The function is expected to return a float. So we should wrap the expression into a function that checks whether the result is a string and in this case converts it into a float? I'll see.

ajdapretnar · 2019-07-19T08:50:21Z

Testing briefly, this is really cool. 👍

lanzagar · 2019-07-19T09:17:48Z

Orange/widgets/data/owfeatureconstructor.py

-        )
+            placeholderText="Expression...",
+            toolTip="Result must be a number for numeric variables, "
+                    "and strings for text variables.\n"


"and a string for text and date/time variables.\n" ?
EDIT: Actually, I see that for datetime vars both are options as well - number or string. Although currently, the number must be a float and not an int, which is a bit awkward.

Maybe it would be even better if subclasses would replace this tooltip with just the information specific to that variable type?

I agreed, implemented it, and no longer agree. It looks visually as the same line edit and the user who will get a tooltip once (and the default is that for numeric attributes, which tells the user to enter a numeric expression) will not expect to get a different tooltip the next time.

Please check and tell me that I can/should remove ac97c90. :)

I agree it is not perfect, but I think removing ac97c90 is worse... The best thing about the tooltip for datetime is that you were able to put in an example, so people actually know what they have to produce in the expression. If those 3 lines are added to the original 4 of the tooltip it becomes a mess.
And anyway, I think the problem would be minimal if when no variable has yet been selected it showed something generic or even no tooltip.
But until the next round of improvements, I would say let's just keep this commit. People not expecting a different tooltip is probably not that big a problem, because you usually need to check it when you have an error in the expression and then you see the relevant information.

Orange/widgets/data/tests/test_owfeatureconstructor.py

lanzagar · 2019-07-19T10:13:49Z

Orange/widgets/data/owfeatureconstructor.py

-        self.valuesedit = QLineEdit()
+        tooltip = \
+            "If values are given, expression must return integer indices.\n" \
+            "Otherwise, expression must return strings."


I guess the third option (using names of values instead of indices or strings) is intentionally not mentioned and is just a bonus hack for power users?
Since it is basically the same as using strings (minus the quotes), we could consider dropping the unmentioned 3rd option and make this tooltip true...

Gladly. I suppose this was so exotic that we don't care about backward compatibility?

I think we can ignore it in this case (not sure it was documented at any time anyway).

lanzagar · 2019-07-19T10:19:57Z

Orange/widgets/data/owfeatureconstructor.py

+        parse = Orange.data.TimeVariable("_").parse
+
+        def cast(e):  # pylint: disable=function-redefined
+            if isinstance(e, float):


Although this is a niche case, if we are supporting numbers I don't see a reason why 1000. (float) would be accepted and 1000 (int) not...
Maybe we should write the documentation together with these changes, so that we are sure what should work and what should not. It adds a nice incentive to make it simple enough that explaining it is not too hard :)

lanzagar · 2019-07-19T11:10:19Z

Orange/widgets/data/owfeatureconstructor.py

+            try:
+                return parse(e)
+            except:  # pylint: disable=bare-except
+                return nan


This allows user mistakes to pass too silently in my opinion. I would expect an error in the widget to be shown if I return a date "9/11/2001", "2019-01-33" or "2019-21-12" etc. Currently it could be that some dates work, but the incorrect just become missing values without any notice.
The documentation should also definitely link to https://en.wikipedia.org/wiki/ISO_8601 since most people don't actually use the iso standard for dates and probably don't know what they should look like.

Not as a part of this PR, but it would be soooo nice if Orange worked with more than one datetime format. Especially when importing from Excel, which pretends to be smart and formats dates in its own way, this would make it a lot easier for people to import datetimes. I assume we can make a simple function that checks the most likely format and applies it to the datetime column... Or actually converts it into ISO, because I suppose nothing would work in the Timeseries add-on otherwise.

I forget why I wanted to catch exceptions here. Removed.

Accepting different date formats indeed belongs to a separate issue.

codecov-io · 2019-07-30T13:26:16Z

Codecov Report

Merging #3936 into master will increase coverage by <.01%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##           master    #3936      +/-   ##
==========================================
+ Coverage   85.12%   85.13%   +<.01%     
==========================================
  Files         378      378              
  Lines       67117    67194      +77     
==========================================
+ Hits        57134    57204      +70     
- Misses       9983     9990       +7

…able types

janezd force-pushed the featureconstructor-datetime branch from 9f99cb4 to f0516f9 Compare July 12, 2019 12:56

janezd mentioned this pull request Jul 12, 2019

[ENH] Feature constructor: deduce discrete values from function #3929

Closed

3 tasks

janezd changed the title ~~[ENH] Allow creation of Date / Time variables in Feature Constructor~~ [ENH] Feature Constructor: Easier categorical features, allow creation of date/time, easier use of string data Jul 12, 2019

lanzagar self-assigned this Jul 19, 2019

lanzagar reviewed Jul 19, 2019

View reviewed changes

janezd force-pushed the featureconstructor-datetime branch from f0516f9 to 09bfee3 Compare July 30, 2019 15:51

janezd added 5 commits July 30, 2019 18:05

OWFeatureConstructor: Deduct discrete values from function

321924c

OWFeatureConstructor: Cast string values to strings

71321cc

Feature Constructor: Add DateTime variable

663fd18

Feature Constructor: Add 'cast' method to 'FeatureFunc'

40511ee

OWFeatureConstructor: Different line edit tooltips for different vari…

ac97c90

…able types

janezd force-pushed the featureconstructor-datetime branch from 09bfee3 to ac97c90 Compare July 30, 2019 16:06

lanzagar merged commit a8f3d78 into biolab:master Aug 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Feature Constructor: Easier categorical features, allow creation of date/time, easier use of string data #3936

[ENH] Feature Constructor: Easier categorical features, allow creation of date/time, easier use of string data #3936

janezd commented Jul 11, 2019 •

edited

Loading

codecov bot commented Jul 11, 2019 •

edited

Loading

ajdapretnar commented Jul 12, 2019

janezd commented Jul 12, 2019

ajdapretnar commented Jul 19, 2019

lanzagar Jul 19, 2019

lanzagar Jul 19, 2019

janezd Jul 30, 2019 •

edited

Loading

lanzagar Aug 2, 2019

lanzagar Jul 19, 2019

janezd Jul 30, 2019

lanzagar Aug 2, 2019

lanzagar Jul 19, 2019

lanzagar Jul 19, 2019

ajdapretnar Jul 19, 2019

ajdapretnar Jul 19, 2019

janezd Jul 30, 2019

codecov-io commented Jul 30, 2019 •

edited by codecov bot

Loading

[ENH] Feature Constructor: Easier categorical features, allow creation of date/time, easier use of string data #3936

[ENH] Feature Constructor: Easier categorical features, allow creation of date/time, easier use of string data #3936

Conversation

janezd commented Jul 11, 2019 • edited Loading

Includes

codecov bot commented Jul 11, 2019 • edited Loading

Codecov Report

ajdapretnar commented Jul 12, 2019

janezd commented Jul 12, 2019

ajdapretnar commented Jul 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janezd Jul 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jul 30, 2019 • edited by codecov bot Loading

Codecov Report

janezd commented Jul 11, 2019 •

edited

Loading

codecov bot commented Jul 11, 2019 •

edited

Loading

janezd Jul 30, 2019 •

edited

Loading

codecov-io commented Jul 30, 2019 •

edited by codecov bot

Loading