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

[FIX] Fix transformation for non primitive variables #1770

Merged
merged 2 commits into from
Nov 22, 2016

Conversation

ales-erjavec
Copy link
Contributor

Issue

When editing a StringVariable in "Edit Domain" widget an TypeError is raised:

Traceback (most recent call last):
  File "/Users/aleserjavec/workspace/orange3/Orange/widgets/gui.py", line 1978, in do_commit
    commit()
  File "/Users/aleserjavec/workspace/orange3/Orange/widgets/data/oweditdomain.py", line 543, in commit
    new_data = self.data.from_table(new_domain, self.data)
  File "/Users/aleserjavec/workspace/orange3/Orange/data/table.py", line 374, in from_table
    is_sparse=sp.issparse(source.metas))
  File "/Users/aleserjavec/workspace/orange3/Orange/data/table.py", line 324, in get_columns
    a[:, i] = match_type(col(source))
  File "/Users/aleserjavec/workspace/orange3/Orange/preprocess/transformation.py", line 27, in __call__
    domain = Domain([self.variable])
  File "/Users/aleserjavec/workspace/orange3/Orange/data/domain.py", line 127, in __init__
    raise TypeError("variables must be primitive")
TypeError: variables must be primitive
Description of changes

Fix base Transformation class to handle the non primitive variable case.

Includes
  • Code changes
  • Tests
  • Documentation

Fix a 'TypeError: variables must be primitive' when a transformation
is defined for a `StringVariable` (as is the case in 'Edit Domain'
widget).

This was broken since 959f696
@janezd
Copy link
Contributor

janezd commented Nov 22, 2016

I fixed the same thing this weekend. :) See dbfa1e2.

The fix is the same, but tests are different. You call an actual transformation and I use a mock and call transform directly. Can you add my tests to your PR -- if you agree they make sense, of course? I would then merge your PR and remove my fix from my (otherwise unrelated) PR.

@codecov-io
Copy link

codecov-io commented Nov 22, 2016

Current coverage is 88.91% (diff: 100%)

Merging #1770 into master will increase coverage by <.01%

@@             master      #1770   diff @@
==========================================
  Files            82         82          
  Lines          8847       8853     +6   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           7866       7872     +6   
  Misses          981        981          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 050004c...65c5d4b

transformation.Transformation: Fix for transformation of non-primitive attributes
@janezd janezd merged commit f5dc821 into biolab:master Nov 22, 2016
@ales-erjavec ales-erjavec deleted the fixes/transformation-non-primitive branch January 27, 2017 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants