-
Notifications
You must be signed in to change notification settings - Fork 309
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
feat: removed pkg_resources from all test files and moved importlib into pandas extra #1726
Conversation
No region tags are edited in this PR.This comment is generated by snippet-bot.
|
4cb86ab
to
43e89a6
Compare
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. Added one observation for potentially unnecessary version checks in test files.
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
@pytest.mark.skipif( | ||
PANDAS_INSTALLED_VERSION >= pkg_resources.parse_version("2.0.0"), reason="" | ||
) | ||
@pytest.mark.skipif(PANDAS_INSTALLED_VERSION[0:2] not in ["0.", "1."], reason="") |
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 has several elements in the list so it is a bit easier to grok what is happening here.
0e8af65
to
c50eca3
Compare
This breaks BigQuery DataFrames e2e tests. Let's revert.
|
Good catch @tswast ! I can produce the issue when using an editable install using egg as it appears in the build log When I install using an editable install using egg, the import fails
When I install without an editable install, the import succeeds
I can produce the same issue with the
but not with
The change to move to native namespace packages was released in July for We should investigate whether we want to block on this issue before reverting the change as we do want to support python 3.12 without using We could also consider the change to move to native namespace packages as a potential breaking change and bump the major version. If this only affects editable installs, IMO we should consider this a minor version instead of a major version bump as there are limitations for editable installations: |
Thanks for the investigation! I think we can remove the "-e" in the BigFrames tests. |
In this PR
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 #1725 🦕
Fixes #313 🦕