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] Widgets update #12

Merged
merged 9 commits into from
Oct 3, 2017
Merged

[FIX] Widgets update #12

merged 9 commits into from
Oct 3, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Oct 2, 2017

Fixes #11.
+.gitignore
+github templates

data = Input("Data", Table)

class Outputs:
data = Output("Matching data", Table)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matching Data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

data = Input("Data", Table)

class Outputs:
matching_data = Output("Matching data", Table)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant the labels. We title case them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@ajdapretnar
Copy link
Collaborator

Thank you for this! I have some extra requests, if possible.

  1. Add a newline break in tooltip, e.g. replace in join. (l:206 in owitemsets.py)
  2. Solve QVariant warnings in console. (if possible)
  3. Add a warning for sparse data. ("Can't induce rules on sparse data (yet).")
  4. Is it possible to fix Find Itemsets/Find Rules so that when checking the box the itemsets/rules are found automatically?

@jerneju
Copy link
Contributor Author

jerneju commented Oct 3, 2017

@ajdapretnar : Ok, but I will open a new PR.

@ajdapretnar
Copy link
Collaborator

Number 2 belongs here I think (error solving). The rest can go into a separate PR.

@kernc
Copy link
Contributor

kernc commented Oct 3, 2017

Agree bullets above are out of scope.

@kernc kernc merged commit 8bbc725 into master Oct 3, 2017
@ajdapretnar
Copy link
Collaborator

Except that QVariant wasn't solved, but who cares...

@jerneju jerneju deleted the widgets-update branch October 3, 2017 11:02
@kernc
Copy link
Contributor

kernc commented Oct 3, 2017

Warning is not an error nor necessarily a bug. If the warning in question is:

Trying to create a QVariant instance of QMetaType::Void type, an invalid QVariant will be constructed

I'm happy to take it in a new PR as it might not be as trivial as this was.

@ajdapretnar
Copy link
Collaborator

#11 refers to warnings, including QVariant. If it is not trivial, this should be stated, since it would seem here that the entirety of #11 was solved (which it wasn't).

@jerneju
Copy link
Contributor Author

jerneju commented Oct 3, 2017

I agree, it is not trivial.

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