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

Adding levels #5789

Open
wants to merge 22 commits into
base: development
Choose a base branch
from
Open

Conversation

JorisGoosen
Copy link
Contributor

@JorisGoosen JorisGoosen commented Feb 11, 2025

As requested by @JohnnyDoorn and many others, here is support for manually adding and removing levels in JASP.

This also adds a button to the filter, this to control whether all levels should be passed on or not.

It does exactly what it says on the tin.

Older jaspfiles loaded will have it on by default, so dropping levels.
While fresh workspaces will keep levels by default so useradded levels arent dropped immediately...

Ive kept the old default behaviour of dropping levels as the default. But to make the transition easier it is only used in R *if there is any row that is filtered out!*.

So, if the filter is still the default, or it is a nonsensical filter that passes everything it will not drop levels that are missing from the data (which would be those manually added by the user).
If the filter has at least one row filtered out it will instead respect the property "dropLevels" which will, if enabled, drop any levels no longer (or never) in the data.

@JorisGoosen
Copy link
Contributor Author

The levels check should now work and get triggered

@boutinb
Copy link
Contributor

boutinb commented Feb 14, 2025

If you have an analysis that displays the levels of a variable, when adding a label, it adds this label in the result. But if you uncheck another label (in the filter column), then this label and the newly created label is removed. I don't think this is the right behavior. I think per default, all the checked labels should be used by the analysis.

I find the 'Only filtered levels' button in the Filter editor quite disturbing: this button has a GUI behavior which is not seen elsewhere in JASP: its tooltip explains what happens if you click on it, not what means 'Only filtered levels'. That it is actually a button is already not trivial, as it is integrated on the bottom bar of the filter editor. I think I would prefer a checkbox 'Keep all levels' with a tootip explaining what it means to have it on or off.
I would propose to disable this Checkbox as long as there is nothing in the Filter, and per default JASP keeps all labels that are not unchecked in the Label editor. If the user sets a filter in the Filter editor, the Checkbox should be enabled, and its default value should be off: only the filtered labels are used. If the user removes the filter, then the checkbox becomes disabled, and again all the 'not-unchecked' labels are used.

@JorisGoosen
Copy link
Contributor Author

Or we just disable it by default and its all clear

@JohnnyDoorn
Copy link
Contributor

OK so I just played around with the latest version and like it!
I think that the drop levels is something that JASP does automatically, and not specified by the user, so I do not like that behavior and think such things should be kept to a minimum. So I would propose to have the default on not dropping the levels on newly added levels (which also sounds more logical - why would you add levels to then immediately want to disregard them from the analysis? at least that should not be the default). So JASP behavior should stay as close to what the user has specified themselves.

So for example, when we have facFive (with some edits), and we add a lvl that denoted missingness (999), I would want to keep all levels by default (after filtering out level 2) in my output:
Screenshot 2025-02-17 at 13 43 48

@boutinb
Copy link
Contributor

boutinb commented Feb 18, 2025

But now, if you filter out one label, the label is still in the table:

Screenshot 2025-02-18 at 11 03 48

You have to go to the filter editor to set it to 'Only filtered levels'.
I think that most people will be confused with this behaviour, and will not see immediately that they have to go to the Filter editor to remove really the label from their results.

@JohnnyDoorn
Copy link
Contributor

JohnnyDoorn commented Feb 18, 2025

OK that's a good point; I see now that the discussion is about what happens after the filter is applied and then I understand that we do droplevels (i.e., the user requests filtering, so we make sure that everything gets filtered properly). I think it makes sense that when a user filters out facFive "2", then there should be no output for facFive "2".

So then the default will stay at "Only filtered levels" when a filter is applied, and "keep all levels" when a filter is not applied? With the user being able to toggle that box whenever they want the other option?

edit:

OK so I would want to propose a middle ground - only droplevels for those levels that have been specifically filtered out by the user. That means, in the screenshot below, that only level 2 should have been dropped, while 6 and 7 (which still have a checkmark) are shown. Is this something that is feasible/reasonable? Because I think that aligns most closely to what the user would expect based on how they have filtered. Also, would it make sense to also have this button (synced) in the variable settings menu where these tickmarks can be placed?
image

@JorisGoosen
Copy link
Contributor Author

only droplevels for those levels that have been specifically filtered out by the user. That means, in the screenshot below, that only level 2 should have been dropped, while 6 and 7 (which still have a checkmark) are shown. Is this something that is feasible/reasonable?

Bruno suggested this too and it sounds reasonable for the label-filters.
It could work and because the "filtering this level" is stored per level this could work.
But then instead of seeing the generated (R) filter with the labels being selected like here:
image
you would then have a special filter builtin to the engine/jasprcpp that would have to do this.
This would require quite some extra code in rbridge/rinterface etc.

And then there is still the problem of what to do about the other filters.
But what about when you make an Rcode or dragndrop filter?
How do I know which labels are "supposed to be filtered out" or not?

I mean, I could completely remove using the function dropLevels and manually construct the factors.
Then we could make it so that only those levels that are in the data and are filtered out are not passed on to the analysis.
But then the manually added levels would still be there. So you would have levels that arent in the data and not in the levels list and levels in the levels-list that were never in the data. but the analysis is still getting them...
This would require reading the data twice, once with filter and once without and then doing all sorts of comparisons...

@JorisGoosen
Copy link
Contributor Author

So then the default will stay at "Only filtered levels" when a filter is applied, and "keep all levels" when a filter is not applied? With the user being able to toggle that box whenever they want the other option?

No, the default is "keep levels".
So if the user filters anything it is removed from the data but it still has the level specified on the factor.

If they dont want that they can drop levels.

@JorisGoosen
Copy link
Contributor Author

Also, would it make sense to also have this button (synced) in the variable settings menu where these tickmarks can be placed?

I can add the same button there somewhere yes.

@JorisGoosen
Copy link
Contributor Author

Maybe its more logical to put the dropping-levels property per column?
Then we could put "drop levels" as default.

But then as soon as someone manually adds a level we can switch to "keep levels" right there and then for that column only.
The user can then see it happen ^^

And Id remove the button from the filterwindow.

@boutinb
Copy link
Contributor

boutinb commented Feb 20, 2025

Maybe its more logical to put the dropping-levels property per column? Then we could put "drop levels" as default.

But then as soon as someone manually adds a level we can switch to "keep levels" right there and then for that column only. The user can then see it happen ^^

I find it a good solution! It makes sense.

@boutinb
Copy link
Contributor

boutinb commented Feb 20, 2025

I found another issue: if you remove one level, it updates the analysis, but if you undo it, it does not update the analysis.

@JohnnyDoorn
Copy link
Contributor

Maybe its more logical to put the dropping-levels property per column? Then we could put "drop levels" as default.

But then as soon as someone manually adds a level we can switch to "keep levels" right there and then for that column only. The user can then see it happen ^^

And Id remove the button from the filterwindow.

love it!

@JorisGoosen
Copy link
Contributor Author

Ok the button works but there is a weird issue with the filter-button not updating correctly. So I guess I should also fix that while im at it

@JorisGoosen
Copy link
Contributor Author

Ok this is ready for review again and Ive also triggered some new builds

@JorisGoosen
Copy link
Contributor Author

Ooh no wait I forgot something

- when disabling a label (filterAllows) and dropLevels==noChoice it becomes drop levels
- when adding a label and dropLevels==noChoice it becomes keep levels
- default when noChoice is drop levels
@JorisGoosen
Copy link
Contributor Author

Alright, now Ive also actually implemented the noChoice->drop/keep behaviour for the button. It is ready for review again

…with those buttons

Also makes tooltips on disabled Rectangular/RoundedButtons show
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