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

Fixed the issues with columns in trajectory table. #71

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

achasmita
Copy link
Contributor

No description provided.

@achasmita
Copy link
Contributor Author

Testing is done and table is loading fine now.
Screen Shot 2023-09-29 at 12 24 06 AM
Screen Shot 2023-09-29 at 12 24 39 AM
Screen Shot 2023-09-29 at 12 25 19 AM
Screen Shot 2023-09-29 at 12 26 28 AM
Screen Shot 2023-09-29 at 12 26 49 AM

@achasmita
Copy link
Contributor Author

I am not able to make filter by date range yet , i will try it in morning again.

@shankari
Copy link
Contributor

shankari commented Sep 29, 2023

@achasmita I think that the code to filter by date range is fairly simple - you should be able to copy it from the trip code

if start_date is not None:

Let's try to get it in before merging because people may want to filter by a date range given how slow the data load is.

@achasmita
Copy link
Contributor Author

I am using the same code, and as table is loaded only when tab is selected, I am still figuring out on making both thing works.

@achasmita
Copy link
Contributor Author

Is it ok if we load a recent week data at first and then allow user to load data based on the date range they select. Instead of loading data only when trajectory table is selected ?

@shankari
Copy link
Contributor

shankari commented Oct 2, 2023

@achasmita are you proposing to roll in the scalability fixes (#51)?
I have no objections to that approach, but would like to understand why it is needed.

As I said before, please record what you tried in the issue and how it failed
"The goal is not just to get things to work, but to get them to work right".
#70 (comment)

@shankari
Copy link
Contributor

shankari commented Oct 2, 2023

@achasmita it will also give me a sense of how complicated the problem is, and whether we should go ahead with pushing this change

@achasmita
Copy link
Contributor Author

@achasmita are you proposing to roll in the scalability fixes (#51)? I have no objections to that approach, but would like to understand why it is needed.

As I said before, please record what you tried in the issue and how it failed "The goal is not just to get things to work, but to get them to work right". #70 (comment)

Yes it will work for both. I tried to pass the date range to filter trajectory table but the table was not loading as expected. It was loading all data everytime. I saw the date range change when i select different date range #72 but the data remains
same.

@achasmita
Copy link
Contributor Author

@achasmita are you proposing to roll in the scalability fixes (#51)? I have no objections to that approach, but would like to understand why it is needed.

As I said before, please record what you tried in the issue and how it failed "The goal is not just to get things to work, but to get them to work right". #70 (comment)

Yes it will work for both. I tried to filter the trajectory table in previous code by date range but there was no change in data even after filter is provided. Everytime all the data is loadaed in table so last time I filed the issue.

@achasmita
Copy link
Contributor Author

Ok I figured it out, I was doing sth wrong while passing the date, now it works both the way either we can load trajectory tab when respective tab is selected and apply filter range or we can load data for current week and use date range to load data as needed.

@@ -4,7 +4,7 @@
The workaround is to check if the input value is None.
"""
from dash import dcc, html, Input, Output, callback, register_page, dash_table

from datetime import date
Copy link
Contributor

Choose a reason for hiding this comment

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

I am merging this for now, but we should revisit the time handling in general since datetime does not handle timezones very well. We should use arrow like the rest of the e-mission codebase, and also consider date formatting etc.

Can you please file a follow-up issue for this?

@shankari shankari merged commit af60fa3 into e-mission:master Oct 2, 2023
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.

2 participants