Skip to content

Conversation

@Castronova
Copy link
Contributor

@Castronova Castronova commented Dec 5, 2024

Changes:

  1. Moved WSW vs Width from the node profile graph to the reach timeseries graph.
  2. Added a function to sort timeseries graph data along the x-axis before its rendered.
  3. Implemented X-sort prior to loading both reach and node charts.
  4. Added check to make sure that charts exist before we try to update their styles. This suppresses an error we get on initial loading of node profile charts.
  5. Changed the "Reach Timeseries" tab to "Reach Averaged"

@Castronova Castronova requested a review from devincowan December 5, 2024 14:16
…de profile charts, added check to make sure charts are not undefined when attempting to update them.
Copy link
Collaborator

@devincowan devincowan left a comment

Choose a reason for hiding this comment

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

This is great @Castronova! Nice!
Two things need to be fixed before we can merge:

  1. Please resolve conflicts with Develop (I checked, doesn't look like this should be too bad)
  2. There is a bug when you change the DQ settings after plotting the WSE/WIDTH
wse-vs-width.mov

Copy link
Collaborator

@devincowan devincowan left a comment

Choose a reason for hiding this comment

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

I didn't look at the code yet, but I see that you resolved the DataQuality bug that I noted on first review 👏

But I see that if I access this plot using a shared link, for example:
https://cuahsi.github.io/SWOT-Data-Viewer/#/plots/62222100021?variables=wse/width
then the plot is pretty funkified:
image

UPDATE: oh interesting, it looks like it is maybe because
https://cuahsi.github.io/SWOT-Data-Viewer/#/plots/62222100021?variables=wse/width
redirects to:
https://cuahsi.github.io/SWOT-Data-Viewer/#/plots/62222100021?variables=wse/time

@devincowan devincowan force-pushed the CAM-461/wse-vs-reach-for-timeseries-view branch from cd7ba11 to 704d0cf Compare March 25, 2025 13:52
@devincowan devincowan force-pushed the CAM-461/wse-vs-reach-for-timeseries-view branch from 704d0cf to 3135b83 Compare March 25, 2025 13:55
@devincowan devincowan force-pushed the CAM-461/wse-vs-reach-for-timeseries-view branch from 3135b83 to 923dbed Compare March 25, 2025 14:02
@devincowan
Copy link
Collaborator

I took a look at this PR again today. Appears it still has the same issue that I mentioned back in December

Here is a video showing the problem
https://github.com/user-attachments/assets/ce531872-1a68-4d34-9b70-d0229cecb55f

@devincowan
Copy link
Collaborator

I tried resolving conflicts against develop for this PR and was not successful.
I'm hoping @Castronova that you will have better success...

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