-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BLD: updated conda recipe to be conda-bld 3plus compliant #18592
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
BLD: updated conda recipe to be conda-bld 3plus compliant #18592
Conversation
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.
lgtm. so we don't actually test this on the ci directly. would be nice to tackle that as well (we would hijack a specific build, like we do for BUILD_TEST, where we use pip to build), and use this recipe to build it directly.
conda.recipe/meta.yaml
Outdated
@@ -14,14 +14,14 @@ requirements: | |||
build: | |||
- python | |||
- cython | |||
- numpy x.x | |||
- numpy | |||
- setuptools | |||
- pytz |
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.
we can remove pytz/python-dateutil from the build step
@jreback Sounds like a plan. I'll check out BUILD_TEST. Separate PR? |
@bobhaffner actually, can you add to this PR? |
@jreback No problem. I'll check it out later today |
Codecov Report
@@ Coverage Diff @@
## master #18592 +/- ##
==========================================
- Coverage 91.44% 91.43% -0.02%
==========================================
Files 157 157
Lines 51379 51379
==========================================
- Hits 46985 46978 -7
- Misses 4394 4401 +7
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18592 +/- ##
==========================================
- Coverage 91.59% 91.43% -0.17%
==========================================
Files 153 157 +4
Lines 51361 51379 +18
==========================================
- Hits 47046 46978 -68
- Misses 4315 4401 +86
Continue to review full report at Codecov.
|
I'm having a little trouble figuring out how to tackle this BUILD_TEST. I'm thinking about adding a piece at line 170 of install_travis.sh to remove the pip build after it's complete and then proceed with the conda recipe build. Not sure though, my CI experience is limited. Eager to learn it though. Any advice would be appreciated. |
don’t remove the existing code rather we can add this for say CONDA_BUILD_TEST would repurpose one of the other allowed failure builds just change the name of that build (eg ci/requirements needs changes as well as .travis.yml) |
Thanks for the reply, @jreback So add a CONDA_BUILD_TEST to the requirements-3.6_CONDA_BUILD_TEST.build I intend to leave the .pip and the .build empty and do the |
don’t create new, repurpose the 3.5 build: https://travis-ci.org/pandas-dev/pandas/jobs/310085793 |
dcdf988
to
bf34fcd
Compare
@@ -0,0 +1 @@ | |||
|
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 file not needed
doc/source/whatsnew/v0.22.0.txt
Outdated
@@ -330,4 +330,5 @@ Other | |||
^^^^^ | |||
|
|||
- Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`) | |||
- | |||
- Updated conda recipe to be in compliance with conda-build 3.0+ (:issue:`18002`) |
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 we now have a section on building changes; don't need the whatsnew about ci
Thanks for the feedback, @jreback |
|
||
conda install conda-build | ||
|
||
conda build ./conda.recipe/ --numpy 1.11 --python 3.6 -q |
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.
rather than the actual build line here, but this in install_travis_script; call this BUILD_TEST_CONDA (rather than BUILD_TEST) so you can disambiguate, rename the other to BUILD_TEST_PIP.
.travis.yml
Outdated
@@ -79,6 +80,9 @@ matrix: | |||
- dist: trusty | |||
env: | |||
- JOB="2.7_SLOW" SLOW=true | |||
- dist: trusty | |||
env: | |||
- JOB="3.6_CONDA_BUILD_TEST" TEST_ARGS="--skip-slow" BUILD_TEST=true |
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.
need a space here, this line has to EXACTLY match the one above. add back --skip-network to TEST_ARGS
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.
don't add this to the allowed failures, it should be a main build test only
Hi @jreback, should I remove BUILD_TEST(soon to be PIP_BUILD_TEST) from allowed failures as well? |
leave these as they r for the location of allowed failures |
8b70712
to
d0c77a6
Compare
Hey @jreback, Its obvious this one is out of my depth so I'm packing it in. I appreciate the advice and the patience. Let me know if you want me to re-submit the original PR |
no prob i will finish this up from here but feel free to work on other issues! |
Thanks, Jeff. And I intend to. Even though I've made just a couple of contributions, I've really enjoyed it and feel like I've learned a lot. |
superseded by #18787 |
git diff upstream/master -u -- "*.py" | flake8 --diff
I did local testing of the build. Assuming the CI will catch any errors?