search

pacificclimate / pdp

The PCIC Data Portal - Server software to run the entire web application
GNU General Public License v3.0
1 stars 2 forks source link

Update canada_ex_app (Downscaled GCM Scenarios) #121

Closed rod-glover closed 5 years ago

rod-glover commented 5 years ago

This PR updates canada_ex_app, which provides the Downscaled GCM Scenarios portals.

This is the main and most complicated app where changes involving date-selection must be done. (Others, simpler, to come in a later PR.)

Overview

Changes to the app

Tests and test support

rod-glover commented 5 years ago

Suggestion for reviewers: In GitHub PR, on Files changed tab, select the gear icon, then select "Hide whitespace changes". It reduces the noise in the diffs.

rod-glover commented 5 years ago

I will shortly put up a Docker instance running the modified code.

rod-glover commented 5 years ago

Demo version now running.

Only the Downscaled GCMs app has been updated. Other apps are probably erroneous now. (But will be fixed in upcoming PR.)

corviday commented 5 years ago

I'm looking at the demo. Is this update solving issue #110 in the downscaled gcms scenarios portal, or is it a step toward that end?

The URL I see when I hover over the download link the portal upon first opening it is https://services.pacificclimate.org/dataportal/data/downscaled_gcms/tasmax_day_BCCAQv2+ANUSPLIN300_CanESM2_historical+rcp26_r1i1p1_19500101-21001231.nc.nc?tasmax[0:55152][0:510][0:1068]& but this file has 55115 timesteps:

ncdump -h tasmax_day_BCCAQv2+ANUSPLIN300_CanESM2_historical+rcp26_r1i1p1_19500101-21001231.nc 
netcdf tasmax_day_BCCAQv2+ANUSPLIN300_CanESM2_historical+rcp26_r1i1p1_19500101-21001231 {
dimensions:
    lon = 1068 ;
    lat = 510 ;
    time = 55115 ;
 variables:
         ...
//global attributes
          ...
}

But if this code isn't actually tackling that issue, obviously this isn't an issue. :)

EDIT: Nevermind, the demo is a little wonky, and this issue will probably go away when the demo is updated.

rod-glover commented 5 years ago

The demo has been fixed (wrong config, argh). @corviday , you were right that what you were seeing was due to that problem. So have at it!

corviday commented 5 years ago

When opening the portal initially or switching to a new file, the end date defaults to the current date. Previously, the end date was the last date available in the dataset. I think having the end date default to the current date makes sense for PCDS and other portals based on real data, but for modeled datasets like the Downscaled GCM Scenarios, having the end date be the last date in the file might be more convenient; I think it's more likely to be what users want.

corviday commented 5 years ago

The demo looks good. I appreciate all the extra work it took to compensate for the missing calendar-style datepicker interface: displaying the start and end dates, error messages when people enter invalid dates, etc. I wasn't able to trip it up despite some efforts to the contrary; all the events seem to fire in the right order.

rod-glover commented 5 years ago

@corviday , I checked that the download link time ranges were correct for a few cases. But since this is basically the core of the issue, would you mind kicking the tires a few times on this if you haven't already?

corviday commented 5 years ago

I have played around with the time ranges a fair bit; they look good to me.

corviday commented 5 years ago

Overall this seems pretty solid to me! I want to look over the test code a bit more in the morning, but everything seems to be in good shape.

rod-glover commented 5 years ago

@corviday , ready for your next review. Thanks for your diligence!

rod-glover commented 5 years ago

There's an intermittent error that looks like it's due a problem in the Google Chrome distro, maybe they didn't update their hash sums, or they are doing some kind of CD and the hash sums are lagging or cached or something (???).

Anyway it's not a fault in our code. If it persists, we should probably file an issue with TravisCI.

Reading package lists...
E: Failed to fetch http://dl.google.com/linux/chrome/deb/dists/stable/main/binary-amd64/Packages.gz  Hash Sum mismatch
E: Some index files failed to download. They have been ignored, or old ones used instead.
The command "sudo apt-get update -q" failed and exited with 100 during .
corviday commented 5 years ago

I ran the frontend tests and got this error:

    console.error node_modules/jsdom/lib/jsdom/virtual-console.js:29
      Error: Not implemented: HTMLCanvasElement.prototype.getContext (without installing the canvas npm package)
          at module.exports (/home/lzeman/Code/pdp-test/pdp/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:9:17)
          at HTMLCanvasElementImpl.getContext (/home/lzeman/Code/pdp-test/pdp/node_modules/jsdom/lib/jsdom/living/nodes/HTMLCanvasElement-impl.js:42:5)
          at HTMLCanvasElement.getContext (/home/lzeman/Code/pdp-test/pdp/node_modules/jsdom/lib/jsdom/living/generated/HTMLCanvasElement.js:50:45)
          at eval (eval at <anonymous> (/home/lzeman/Code/pdp-test/pdp/pdp/static/js/__test__/globals-helpers.js:68:13), <anonymous>:4082:39)
          at eval (eval at <anonymous> (/home/lzeman/Code/pdp-test/pdp/pdp/static/js/__test__/globals-helpers.js:68:13), <anonymous>:4083:3)
          at eval (<anonymous>)
          at globalEval (/home/lzeman/Code/pdp-test/pdp/pdp/static/js/__test__/globals-helpers.js:68:13)
          at Array.forEach (<anonymous>)
          at Object.forEach [as importGlobals] (/home/lzeman/Code/pdp-test/pdp/pdp/static/js/__test__/globals-helpers.js:58:11)
          at Object.importGlobals (/home/lzeman/Code/pdp-test/pdp/pdp/static/js/__test__/canada_ex_app.test.js:5:30) undefined

Is that expected and generally known to be harmless?