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

37 save data between views use scenarios #40

Merged
merged 6 commits into from
May 1, 2024

Conversation

amynickolls
Copy link
Contributor

Created both Session and Saved Scenarios. Session scenarios are how we will transfer data with the user between views. In this implementation it saves the historic data filters, but it will also save any adjustments or prediction inputs as we develop those pages.

Session scenarios are created/accessed through a router_handler that redirects to the relevant URL.

Dashboard page and relevant code has been deleted as this is not in the designs and was complaining re: the updated scenario model.

@amynickolls amynickolls linked an issue Apr 25, 2024 that may be closed by this pull request
5 tasks
Copy link
Member

@tab1tha tab1tha left a comment

Choose a reason for hiding this comment

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

  • When filters such as Placement_type=Residential and Age=10 to 16 are added to the historic data tab and then the user clicks away from the tab, the filters remain in place when the user comes back to the tab. This isn't possible on the main branch.
    So the changes in this pull request successfully make filters persist.

@tab1tha
Copy link
Member

tab1tha commented Apr 30, 2024

Other notes
About this pull request.

  • I wasn't sure where to click in order to save a scenario.
  • When the user filters the historic view to only see 10-16 year-olds in residential placement. Should the plots in the prediction graph only be those which are relevant to the filters in the historic data tab?. Now, the prediction graph seems to remain unchanged irrespective of the historic data filters.

In general

  • When I Empty cache and hard reload the browser page (to delete cookies), the application keeps me logged on. Flagging this because it might be expected that the user is logged out when cookies are cleared. It is also possible that my cookies are not being cleared in the way that I think so feel free to ignore this if it works okay on your end.
  • It could be nice if the scenario view page had a button from where the user could run the scenario. (this is a screenshot from the scenarios page in the main branch. Didn't get to try out the one in this branch since I couldn't successfully save a scenario so feel free to ignore this if that feature has already been built)
image

@amynickolls
Copy link
Contributor Author

Thanks Tambe! All expected for now. Both saving and loading saved scenarios are coming in an issue down the line. Likewise the historic filters will affect the prediction page but this is yet to be implemented.

As for the log in issue might be that you need to close page then clear cache then return?

Copy link

@cyramic cyramic left a comment

Choose a reason for hiding this comment

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

Happy to approve, but a few questions first

  • Why are the print() statements there? Would they be better to remove or to switch to logging statements?
  • Is deleting Scenario and adding SavedScenario, do we need to worry about the migration of data?


# print(pop.stock)
# print(dc.enriched_view)
session_scenario = get_object_or_404(SessionScenario, pk=1)
print(session_scenario.historic_filters)
Copy link

Choose a reason for hiding this comment

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

Is there any reason for the print statements? They tend to get lost. I'd either remove these or use the logging library to log them in a consistent way that can be better captured.

Copy link

Choose a reason for hiding this comment

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

Also, remove the commented ones as well. If you need them, they're still in the git history.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The print statements in sandbox are only for my own testing/debugging, the whole thing will be deleted once developed. Is there a better way to do that? I want to keep the commented code just for returning to.

@@ -0,0 +1,153 @@
# Generated by Django 4.2.10 on 2024-04-24 15:30
Copy link

Choose a reason for hiding this comment

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

Is this an appt hat's already live? Will deleting the scenario table and creating a new table without migrating the data be an issue? Don't worry if not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not live no!

@amynickolls amynickolls merged commit 4c0832d into main May 1, 2024
1 check passed
@amynickolls amynickolls deleted the 37-save-data-between-views---use-scenarios branch May 1, 2024 14:21
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.

Save data between views - use Scenarios
3 participants