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

500 from lineplot when a proportion value is unknowingly filtered out #157

Closed
bobular opened this issue Apr 14, 2022 · 15 comments · Fixed by #177
Closed

500 from lineplot when a proportion value is unknowingly filtered out #157

bobular opened this issue Apr 14, 2022 · 15 comments · Fixed by #177
Assignees
Labels
bug Something isn't working high priority

Comments

@bobular
Copy link
Member

bobular commented Apr 14, 2022

GEMS1
Line plot
x-axis: Age group
y-axis: Case or Control case/case+control
overlay: Lost earnings due to care

or Borrowing, or is seems any variable uner Current care expenses.

image

What does work?

As above but with (three scenarios listed below)

  • y-axis: numeric variable, like Age.
  • y-axis: Participant->Sex female/male+female
  • overlay: Participant->Sex

It's not obvious to me what the pattern is!
I couldn't get a similar 500 in other visualizations using the Lost earnings due to care overlay

@bobular bobular added the bug Something isn't working label Apr 14, 2022
@danicahelb
Copy link

danicahelb commented Apr 14, 2022

overlaying "Lost earnings due to care" works when a different y-axis variable is selected. See:
image

@danicahelb
Copy link

danicahelb commented Apr 14, 2022

If I add a filter to only include data where lost earnings due to care = yes, then the plot is empty. I would expect the plot to still have the same blue line

image

@danicahelb
Copy link

danicahelb commented Apr 14, 2022

I can also see that only cases have data remaining when i filter by "Lost earnings due to care" = yes. maybe this has something to due with the error? (Actually, I can see from the definition that "Lost earnings due to care" was only assessed for cases)

image

@danicahelb
Copy link

I also get the error message when i add "Sunken eyes" as my overlay. Sunken eyes was also only assessed for cases

image

@danicahelb danicahelb added the b57 label Apr 14, 2022
@dmfalke
Copy link
Member

dmfalke commented Apr 15, 2022

Looks like this is probably a backend issue, so moving it to EdaDataService

@asizemore
Copy link
Member

Error is in plot.data. Moving there!

@asizemore asizemore transferred this issue from VEuPathDB/EdaDataService Apr 15, 2022
@asizemore asizemore self-assigned this Apr 15, 2022
@asizemore
Copy link
Member

Pushed to b58. Issue is that there is no overlay data for Control participants, which means the proportion is basically Case/Case. Our plan is to still err, but return a nice error to the user. Will require work from front and back end.

@danicahelb
Copy link

link to 4/15 convo in Slack #eda-map-client-imp
https://epvb.slack.com/archives/C012ZK4Q5CZ/p1650035476658289

@danicahelb danicahelb added bug Something isn't working and removed bug Something isn't working b58 labels May 18, 2022
@danicahelb
Copy link

danicahelb commented Jun 1, 2022

@asizemore this is the ticket we discussed in 6/1 data viz meeting.
we've decided that -- for proportions -- we will allow numerator=denominator and we’ll return all 1s.

@danicahelb danicahelb changed the title 500 from lineplot overlay with variables in Participant-->Current care expenses 500 from lineplot when numerator and denominator for the proportion are the same Jun 1, 2022
@asizemore
Copy link
Member

@danicahelb the issue in this ticket is that the 500 is being thrown not because the num and denom are the same but because there are no samples left with the value of Control. Effectively the lack of data makes it Case/Case, but the user did not choose that. It's a very rare situation. Still, feels like we should somehow alert the user that they subsetted away an entire portion of the denominator? Without an alert, the user would just see a line plot with 1s and nulls and not necessarily know why.

In general being able to select the same num and denominator (for example in the screenshot below) is a different issue. I think the below is more what we talked about at the data viz meeting last week and i can implement a fix if it's a must have for b58.

Screen Shot 2022-06-10 at 11 40 23 AM

@asizemore asizemore changed the title 500 from lineplot when numerator and denominator for the proportion are the same 500 from lineplot when a proportion value is unknowingly filtered out Jun 10, 2022
@danicahelb
Copy link

Hey @asizemore
as we discussed in the 6/13 EDA UX meeting, if the user filters out data that effectively causes the numerator and denominator to be the same, please do not include an error message and allow the y-axis proportion to equal 1 (so there will be a straight line at y=1, which will give the user a clue that something is up).

Lets start here for b58, if we get feedback that it is confusing we can address this confusion later. thanks!

@asizemore
Copy link
Member

Sounds good @danicahelb . This starting point should be addressed by the linked PR.

@asizemore
Copy link
Member

Deployed to qa and feat. moving to ready for testing

@sweverschulman
Copy link

Functionality that when user makes the proportion = 1 to no throw an error but to plot y = 1 seems to be working. Functionality was also tested with PERCH study.

@moontrip
Copy link

@sweverschulman Thank you for your tests and confirmations! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants