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.

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

Test that the top-level package can be imported #1165

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tadamcz
Copy link
Contributor

@tadamcz tadamcz commented Jan 20, 2025

hopefully this would help catch things like #1164 ? (I need to check that this is actually true, i.e. the test actually fails)

@tadamcz
Copy link
Contributor Author

tadamcz commented Jan 20, 2025

To my puzzlement, the test passes?! Even though I am downstream of d865444, where I encountered the bug in #1164.

https://github.com/UKGovernmentBEIS/inspect_ai/actions/runs/12876243894/job/35898928948?pr=1165#step:6:83

What am I doing wrong?

@tadamcz
Copy link
Contributor Author

tadamcz commented Jan 20, 2025

The identical test fails (correctly, I believe) when I install Inspect d865444 in an environment on my laptop 🤔

What am I missing @jjallaire? Is hynek/build-and-inspect-python-package@v2 doing some nonstandard stuff?

@jjallaire
Copy link
Collaborator

I am not sure. I can't get the import inspect_ai to fail for me either (even before your change).

So it could be that there are contexts/environments that are sensitive to the __init__.py file and others that are not?

@tadamcz
Copy link
Contributor Author

tadamcz commented Jan 20, 2025

Pretty sure __init__.py is required as part of the Python spec:

The __init__.py files are required to make Python treat directories containing the file as packages (unless using a namespace package, a relatively advanced feature)

https://docs.python.org/3/tutorial/modules.html#packages

So anything that does not require it would be 'wrong', I believe. (I believe some IDEs are super-permissive when importing things, but I don't think this kind of thing should affect CI.)

@tadamcz
Copy link
Contributor Author

tadamcz commented Jan 20, 2025

I can't get the import inspect_ai to fail for me either

Well, I can get it to fail when I install Inspect (from source) in a new environment

@jjallaire
Copy link
Collaborator

Well, I can get it to fail when I install Inspect (from source) in a new environment

Okay, good. No idea why it was working for me then. Obviously we are better off with the __init__.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants