-
Notifications
You must be signed in to change notification settings - Fork 285
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
Sampledata #19
Sampledata #19
Conversation
still need to change usage after load
Docs are being built with no errors but some work to do. Extests failing with image checksums (as with master), not yet investigated these further.
@@ -133,19 +133,19 @@ matching :meth:`name <iris.cube.Cube.name>` will be returned:: | |||
To constrain the load to multiple distinct constraints, a list of constraints can be provided. | |||
This is equivalent to running load once for each constraint but is likely to be more efficient:: | |||
|
|||
filename = iris.sample_data_path('PP', 'ukV2', 'THOxayrk.pp') | |||
filename = iris.sample_data_path('PP', 'ukV2', 'THOxayrk_subset_subset.pp') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..._subset_subset
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo. will fix, thanks.
doesn't cause an error in make html or make extest,
which is the current focus, but should have been spotted.
That's a really noisy history you've built up (i.e. lots of files get added then removed again). It'll need cleaning up before it can be merged (e.g. cherry-pick or "squash"). |
@@ -49,10 +49,10 @@ def cop_metadata_callback(cube, field, filename): | |||
|
|||
def main(): | |||
# Load data into three Cubes, one for each set of PP files | |||
e1 = iris.load_strict(iris.sample_data_path('PP', 'A1B-Image_E1', 'E1', '*.pp'), | |||
e1 = iris.load_strict(iris.sample_data_path('PP', 'A1B-Image_E1', 'E1_subset', '*.pp'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this has been sub-setted too much. Scientifically, it is not an appropriate representation of the data that it claims to be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion from Phil: why not leave the temporal resolution (i.e. all 240 years) but subset spatially. e.g. You could limit it to northern Europe and the files would shrink dramatically. NB. You'd have to update the corresponding description, plot title, etc. to replace references to global with your chosen region.
Some users are not going to need/want the examples, so we should be aiming to supply separate downloads for the code & the examples. Similarly, we don't really want to burden our code repo with great lumps of example data. |
All issues above have been addressed except for the "cherry-pick or squash" request. |
""" | ||
if not os.path.exists(config.SAMPLE_DATA_DIR): | ||
# TODO: Update this link when it's in SciTools. | ||
warnings.warn("The sample data folder was not found (available from git@github.com:bblay/iris_sample_data.git)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to be SciTools links, and they should be to a download - not a repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This warning should really be an error. And it'd be even better if the check used os.path.isdir()
.
Not sure whether to abandon this pull request, as pelson has taken it on? |
This PR is made obsolete by #34. Closing for now, can re-open if something comes up. |
Sample data currently at 14MB.
Solves #2.