Uh oh!
There was an error while loading. Please reload this page.
test: ensure prerelease versions of pandas and arrow are tested nightly - #961
Conversation
tswast
commented
Sep 10, 2021
Local output, verifying expected package versions are actually used now: |
tswast
commented
Sep 10, 2021
Internal change 396006532 pending review to actually enable this continuous job. |
plamut
left a comment
There was a problem hiding this comment.
Looks good, just had a comment about possibly improving/simplifying pattern matching.
| deps = [ | ||
| line.strip() | ||
| for line in constraints_text.split("\n") | ||
| if line.strip() and line[0] != "#" |
There was a problem hiding this comment.
What if we somehow end up with a comment line that starts with whitespace? Unlikely, indeed, but it would feel clumsy if we accidentally break the pipeline with redundant whitespace in a text file. 😆
BTW, should we instead match the entire lines containing version pins and directly extract the dependency name with grouping in the regex pattern itself? Feels more straightforward, to me at least.
Edit: Something like this maybe (untested):
\s*(\S+)(?===\S+) # .group(1) is the dependency nameThere was a problem hiding this comment.
I like it! I made one modification, which was to add ^ to only match from the start of a line. Otherwise, it picked up "foo" from the comments.
| session.install("-e", ".[all]") | ||
| with open( | ||
| CURRENT_DIRECTORY / "testing" / "constraints-3.6.txt", encoding="utf-8" |
There was a problem hiding this comment.
Oh, BTW, we might not want to hardcode the 3.6 Python version, but instead use UNIT_TEST_PYTHON_VERSIONS[0], i.e. the oldest one supported?
There was a problem hiding this comment.
It's a bit odd to use UNIT_TEST_PYTHON_VERSIONS in something that's a system test, but I agree it's worth having. I added a comment explaining why it's that version.
tswast
commented
Sep 13, 2021
Expected pre-release dependencies are being used: |
plamut
commented
Sep 14, 2021
@tswast Do we first need to wait for the internal change to be applied, or can we already go ahead and merge this? |
tswast
commented
Sep 14, 2021
We need to merge this first, actually. |
…ly (googleapis#961) * test: ensure prerelease versions of pandas and arrow are tested nightly * use regex to find package names rather than filter out comment lines
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes#882 🦕