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

Add a dropdown to select CMIP version #72

Merged
merged 5 commits into from
Dec 17, 2024
Merged

Add a dropdown to select CMIP version #72

merged 5 commits into from
Dec 17, 2024

Conversation

corviday
Copy link
Contributor

@corviday corviday commented Dec 6, 2024

Adds a dropdown to switch between CMIP5 and CMIP6 data. The switch is performed by filtering location data before it is passed to the table.

Demo.

Resolves #71

@corviday corviday requested a review from rod-glover December 6, 2024 20:20
@corviday
Copy link
Contributor Author

corviday commented Dec 9, 2024

Feedback from Stephen:

  • Time period selector does not appear to do anything
  • Could time periods be listed sequentially?
  • Could files be sorted by SSP/RCP, then by time period?

Copy link
Contributor

@rod-glover rod-glover left a comment

Choose a reason for hiding this comment

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

Coming along nicely! Some comments to do with layout and code style, but no functional changes to suggest.

I'll reply as I can to the feedback items from Stephen.

src/components/app-root/AppBody/AppBody.js Outdated Show resolved Hide resolved
src/components/app-root/AppBody/AppBody.js Outdated Show resolved Hide resolved
src/components/controls/VersionControl/VersionControl.js Outdated Show resolved Hide resolved
src/components/controls/VersionControl/VersionControl.js Outdated Show resolved Hide resolved
src/components/controls/VersionControl/VersionControl.js Outdated Show resolved Hide resolved
src/components/controls/VersionControl/VersionControl.js Outdated Show resolved Hide resolved
@rod-glover
Copy link
Contributor

rod-glover commented Dec 10, 2024

Feedback from Stephen:

  • Time period selector does not appear to do anything

It does "do something", but our data is such that it has it nil effect. Okay, confusing.

The selector allows to select for locations that definitely have the specified time period. (As opposed to locations with any time period available, i.e., no filtering. So come to think of it, "Any" would be a much better label than "All".)

Because every location has every time period (it appears), the selector doesn't have any perceptible effect. It may not have much use if this is always true; we should consult with Stephen about that.

  • Could time periods be listed sequentially?

What does that mean? If he's talking about the table of files that shows when you click show, you can click on any column header to sort table by that value. That makes it "sequential" in some sense.

  • Could files be sorted by SSP/RCP, then by time period?

Yes, click on Time Period column, then on the Scenario Column.

What he might prefer is to have the default sort set up this way. That is easy if it is the requirement. Right now it is either not set at all or is the Type column.

@corviday
Copy link
Contributor Author

corviday commented Dec 11, 2024

Could time periods be listed sequentially?

What does that mean? If he's talking about the table of files that shows when you click show, you can click on any column header to sort table by that value. That makes it "sequential" in some sense.

I believe Stephen would like the decades in the rows under the "Time Periods" column to be ordered so it's easier to tell at a glance which ones are present. I am going to look into whether this is easy; if not it can be a separate issue.

Could files be sorted by SSP/RCP, then by time period?

Yes, click on Time Period column, then on the Scenario Column.
What he might prefer is to have the default sort set up this way. That is easy if it is the requirement. Right now it is either not set at all or is the Type column.

I'll see if I can set the default to be scenario for the inner files table.

@corviday
Copy link
Contributor Author

Made suggested code improvements.

Addressed two of Stephen's requests:

  • decades appear in order in the "Time Periods" column of the location table
  • file table is initially sorted by scenario, then time period

Demo has been updated.

@corviday corviday requested a review from rod-glover December 12, 2024 21:39
@rod-glover
Copy link
Contributor

Looks great so far.

@corviday corviday merged commit 6f2afed into master Dec 17, 2024
2 checks passed
@corviday corviday deleted the versions branch December 17, 2024 01:14
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.

Handle multiple versions of weather files
2 participants