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

Import custom case counts #381

Merged
merged 32 commits into from
May 8, 2020
Merged

Import custom case counts #381

merged 32 commits into from
May 8, 2020

Conversation

whiver
Copy link
Contributor

@whiver whiver commented Apr 3, 2020

Related issues and PRs

Description

The goal is to allow the user to import a custom data file instead of the predefined data in the "Confirmed cases" field.

This allows to compare simulated data with real world ones, specific to each organization.

Impacted Areas in the application

Mainly the population card, where the "confirmed cases" dropdown now has a button to display the import popup.

The imported file is stored in the sessionStorage for now to avoid keeping the whole file in the state, and to keep the file between subsequent page refresh, without keeping it forever.

Testing

  • Create a tsv file with these values for example:

    time	cases	deaths	hospitalized	icu	recovered
    

4/24/2020 100 10 50 5 15

  • Under the "Confirmed cases" field, click the import link and import your data
  • Click "Run"

@vercel
Copy link

vercel bot commented Apr 3, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/covid19-scenarios/covid19-scenarios/4bb7fa4tf
✅ Preview: https://covid19-scenarios-git-fork-whiver-feat-import.covid19-scenarios.now.sh

@ivan-aksamentov ivan-aksamentov added pr: work in progress This is work in progress, do not merge until finished t:feat Type: request of a new feature, functionality, enchancement labels Apr 3, 2020
@rneher
Copy link
Member

rneher commented Apr 4, 2020

Thanks @whiver. this doesn't seem to work atm for me, but the general direction is good I think. The main aim of the feature will be to allow individuals hospitals, counties, cities etc to upload their own case counts. It is therefore not so much the simulation output that we want to upload again, but custom case counts (the dots on the graph). The format of the files should be like those that we download though.

@whiver
Copy link
Contributor Author

whiver commented Apr 4, 2020

Thanks @whiver. this doesn't seem to work atm for me, but the general direction is good I think. The main aim of the feature will be to allow individuals hospitals, counties, cities etc to upload their own case counts. It is therefore not so much the simulation output that we want to upload again, but custom case counts (the dots on the graph). The format of the files should be like those that we download though.

Thanks for the confirmation :)
The current version is not working indeed, it's still a WIP, I only created the PR for visibility. I'm on it!

@whiver
Copy link
Contributor Author

whiver commented Apr 7, 2020

Thanks @whiver. this doesn't seem to work atm for me, but the general direction is good I think. The main aim of the feature will be to allow individuals hospitals, counties, cities etc to upload their own case counts. It is therefore not so much the simulation output that we want to upload again, but custom case counts (the dots on the graph). The format of the files should be like those that we download though.

Hi @rneher, @ivan-aksamentov , I now have a first working version! I got a couple of questions too:

  • As we want to import the confirmed cases from the file, I guess that if a file is imported, we display the imported data instead of the default "Confirmed cases" data, right? I added a toggle button to switch between imported/default confirmed cases

  • UI-related question: if we're going to replace the data from "Confirmed cases" with the ones imports from the file, shouldn't we display the import option in the "Simulation" pane with the "Confirmed cases" option? I'm still struggling to find a good UX for this feature so I was wondering about that.

  • The exported file only contains the simulation results (displayed as lines in the graph) and not the confirmed cases (displayed as dots). This means that I had to perform a kind of mapping between the existing fields in the exported file and the scattered dots in the graph. More precisely:

    • infectious in the file becomes Cumulative confirmed cases in the graph
    • cumulative_fatality in the file becomes Cumulative confirmed deaths in the graph
    • severe in the file becomes Patients in hospital in the graph
    • ICU in the file becomes Patients in ICU in the graph

    Is this kind of mapping what you expected? Of course we may add more dot types to the graph later, but I'll first start with the existing ones.

I wanted to make sure that I was heading to the right direction. Thanks in advance for the feedback :)

@whiver whiver marked this pull request as ready for review April 8, 2020 09:03
@whiver whiver mentioned this pull request Apr 16, 2020
@codecov
Copy link

codecov bot commented Apr 17, 2020

Codecov Report

Merging #381 into master will decrease coverage by 1.30%.
The diff coverage is 0.68%.

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   25.91%   24.61%   -1.31%     
==========================================
  Files         120      127       +7     
  Lines        2616     2759     +143     
  Branches      370      391      +21     
==========================================
+ Hits          678      679       +1     
- Misses       1938     2080     +142     
Impacted Files Coverage Δ
src/algorithms/utils/exceptions.ts 0.00% <0.00%> (ø)
src/algorithms/utils/importedData.ts 0.00% <0.00%> (ø)
src/components/Form/FormCustom.tsx 0.00% <0.00%> (ø)
src/components/Main/Compare/FileUploadZone.tsx 0.00% <0.00%> (ø)
.../components/Main/Results/ImportCaseCountDialog.tsx 0.00% <0.00%> (ø)
...ponents/Main/Scenario/ConfirmedCasesDataPicker.tsx 0.00% <0.00%> (ø)
...omponents/Main/Scenario/ScenarioCardPopulation.tsx 0.00% <0.00%> (ø)
src/components/Misc/Message.tsx 0.00% <0.00%> (ø)
src/components/Main/state/getCaseCounts.ts 63.63% <12.50%> (-16.37%) ⬇️
... and 6 more

@whiver
Copy link
Contributor Author

whiver commented Apr 17, 2020

@nnoll I updated the PR to move the import button to the population section. The UI is not awesome, but tweaking this part involves creating a custom form field, so I'd prefer to improve the design in a separate PR, except if you think it's really needed.

I updated the import schema to follow existing cases data files from the data folder. You can test importing for example data/case-counts/brazil/BRA-Acre.tsv. I tried to make the import more resilient to errors.

I still need to find out why on the environment deployed by Now, when you import a file in the popup, its name/size are missing (it displays (NaNKb)), while in local env it works correctly. I would also add an explanation in the import popup about the expected import format.

Feel free to test and tell me what you think! 😃

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Apr 18, 2020

@whiver If you rebase on (or merge) master, with #591 you should be able to debug the production deployments in Chromium-based browsers as follows:

  • in dev tools' "Source" tab, in the sidebar, click "Filesystem", then "+" button. Add project's root directory. Then click "Allow" when asked to give access. You now should be able to open files in the sidebar, set breakpoints and use debug statements, watch call stack and variables. Sometimes it gets wonky - refresh the page and/or restart dev tools or the entire browser in this case.

Don't hesitate to commit with debug or console.log statements, if it helps debugging. We just need to make sure to remove all that before we merge to master.

@whiver
Copy link
Contributor Author

whiver commented Apr 20, 2020

Thanks @ivan-aksamentov, I fixed the issue in prod. I don't exactly understand why, but {[...uploadedFiles.values()].map(({ name, size }: File) => was returning undefined name & size in prod but not in debug.

It should be working now, tell me what do you think of it :)

@whiver whiver changed the title [WIP] Import & compare results data Import custom case counts Apr 21, 2020
@codeclimate
Copy link

codeclimate bot commented May 8, 2020

Code Climate has analyzed commit 20ceb74 and detected 0 issues on this pull request.

View more on Code Climate.

@ivan-aksamentov
Copy link
Member

@whiver Hi William

Sorry for the delay. Have been busy on other stuff.
I have rebased it and am merging this. Thanks for the contribution!

@ivan-aksamentov ivan-aksamentov merged commit 36a3d0d into neherlab:master May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: work in progress This is work in progress, do not merge until finished t:feat Type: request of a new feature, functionality, enchancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants