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.

Sign up for GitHub

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

Jump to bottom

BLD: Pinned cftime 1.0.1 #3247

Merged
merged 1 commit into from
Feb 1, 2019
Merged

BLD: Pinned cftime 1.0.1 #3247

merged 1 commit into from
Feb 1, 2019

Conversation

cpelley
Copy link

@cpelley cpelley commented Jan 7, 2019 •
edited
Loading

@cpelley cpelley requested a review from bjlittle January 7, 2019 16:04
@rcomer
Copy link
Member

rcomer commented Jan 8, 2019

My user code is failing with cftime version 1.0.0b1 so I am not sure if <1.0.2.1 will solve all problems (although getting Travis checks to pass is obviously desirable!) What version would Travis be picking up here? I'm sure that info must be in the output somewhere but I can't see it!

@cpelley
Copy link
Author

cpelley commented Jan 8, 2019

Travis is using cftime 1.0.1 (conda-forge)

@pp-mo
Copy link
Member

pp-mo commented Jan 8, 2019 •
edited
Loading

cftime version 1.0.0b1 ... What version would Travis be picking up here? I'm sure that info must be in the output somewhere but I can't see it!

You can find this from a Travis log :
To view a log, first click on the tests icon by the commit in Github (green tick or red cross!),
navigate to the "continuous-integration/travis-ci/pr ... Details".
On the test page, hit another tick (or cross) for the log of one testing "phase"
(e.g. python 2.7, TEST_TARGET=default, TEST_MINIMAL=True).

In the log, look for the line "$ conda list -n $ENV_NAME ....",
the initial wedge ">" opens the collapsed command output ...
which is the list of installed packages !!

In this case, it has "cftime 1.0.1"

@rcomer
Copy link
Member

rcomer commented Jan 8, 2019

Thanks both. I hadn't even twigged that there was collapsed output in the Travis log!

bjlittle
bjlittle reviewed Jan 8, 2019
View reviewed changes
requirements/core.txt Outdated
@@ -6,7 +6,7 @@
cartopy
#conda: proj4<5
cf_units>=2
cftime!=1.0.2.1
cftime<1.0.2.1
Copy link
Member

@bjlittle bjlittle Jan 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@cpelley Why not pin directly to 1.0.1, just to make this explicit and clear.

We still need to fix iris to deal with 0 based dates, which are now illegal in the latest cftime, and hence the root cause of current master failures.

So this pin would only be a temporary pin...

Copy link
Author

Choose a reason for hiding this comment

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

...Why not pin directly...

Sure.

We still need to fix iris to deal with 0 based dates...

Agreed. Some relating discussion in #3239

@cpelley cpelley changed the title BLD: Pinned cftime <1.0.2.1 BLD: Pinned cftime 1.0.1 Jan 8, 2019
@bjlittle bjlittle self-assigned this Jan 8, 2019
@bjlittle
Copy link
Member

bjlittle commented Jan 8, 2019

@cpelley Apologies. Sorry to be a pain...

But could you please target the 2.2.x branch instead of master, and then I'll merge back the 2.2.x branch to master...

@bjlittle bjlittle added this to the v2.2.1 milestone Jan 8, 2019
@cpelley cpelley changed the base branch from master to v2.2.x January 8, 2019 12:43
@SciTools-assistant SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Jan 8, 2019
@SciTools-assistant SciTools-assistant removed the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Jan 8, 2019
@cpelley
Copy link
Author

cpelley commented Jan 8, 2019

NP :)

bjlittle
bjlittle reviewed Jan 9, 2019
View reviewed changes
requirements/core.txt Outdated
@@ -6,7 +6,7 @@
cartopy
#conda: proj4<5
cf_units>=2
cftime!=1.0.2.1
cftime>=1.0.1,<=1.0.1
Copy link
Member

@bjlittle bjlittle Jan 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@cpelley Surely cftime==1.0.1 is what you mean, right? Doesn't that work instead of >=1.0.1,<=1.0.1...

Copy link
Author

Choose a reason for hiding this comment

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

👍

@pp-mo pp-mo mentioned this pull request Jan 31, 2019
Merged
2 tasks
@bjlittle
Copy link
Member

bjlittle commented Feb 1, 2019

Thanks @cpelley 👍

Ping @pp-mo

@bjlittle bjlittle merged commit b0d5512 into SciTools:v2.2.x Feb 1, 2019
@pp-mo pp-mo mentioned this pull request Feb 1, 2019
Merged
bjlittle pushed a commit that referenced this pull request Feb 1, 2019
@pp-mo @bjlittle
Mergeback 2.2.x (#3256)
6a4f9dc 
* BLD: Pinning cftime to 1.0.1 (#3247)

* Fix for numpy 1v16. (#3257)
@cpelley cpelley deleted the PINNING_CFTIME branch February 4, 2019 15:14
@pp-mo pp-mo mentioned this pull request Feb 20, 2019
Closed
znicholls pushed a commit to znicholls/iris that referenced this pull request Jun 15, 2019
@pp-mo @znicholls
Mergeback 2.2.x (SciTools#3256)
26c9ca7 
* BLD: Pinning cftime to 1.0.1 (SciTools#3247)

* Fix for numpy 1v16. (SciTools#3257)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjlittle bjlittle bjlittle left review comments

Assignees

@bjlittle bjlittle

Labels
Release: Patch Type: Bug
Projects
None yet
Milestone
v2.2.1
Development

Successfully merging this pull request may close these issues.
5 participants
@cpelley @rcomer @pp-mo @bjlittle @SciTools-assistant