Skip to content

Avoid runtime dependency on numpy when importing arro3.core.types#210

Merged
kylebarron merged 2 commits intokylebarron:mainfrom
3ok:avoid-numpy-runtime-dep
Oct 2, 2024
Merged

Avoid runtime dependency on numpy when importing arro3.core.types#210
kylebarron merged 2 commits intokylebarron:mainfrom
3ok:avoid-numpy-runtime-dep

Conversation

@3ok
Copy link
Copy Markdown
Contributor

@3ok 3ok commented Oct 2, 2024

By building from source or installing the most recent pre-release on an empty virtual environment.

from arro3.core.types import ArrowSchemaExportable
# ModuleNotFoundError: No module named 'numpy'

Actually importing numpy may not be needed and we can get by with just a if TYPE_CHECKING condition and a forward reference.

@kylebarron
Copy link
Copy Markdown
Owner

Oops, this was unintentional! Thank you!

I was tired and forgot momentarily that types.py was actually imported by Python and not just by the type checker


# Numpy arrays don't yet declare `__buffer__` (or maybe just on a very recent version)
ArrayInput = Union[ArrowArrayExportable, BufferProtocolExportable, np.ndarray]
ArrayInput = Union[ArrowArrayExportable, BufferProtocolExportable, "np.ndarray"]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since we use from __future__ import annotations above, we don't need to put this in a string

Suggested change
ArrayInput = Union[ArrowArrayExportable, BufferProtocolExportable, "np.ndarray"]
ArrayInput = Union[ArrowArrayExportable, BufferProtocolExportable, np.ndarray]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes I thought the same but actually you still need it for aliases, otherwise I get this

>>> from arro3.core.types import ArrowSchemaExportable
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/redab/tmp/venv/lib/python3.10/site-packages/arro3/core/types.py", line 82, in <module>
    ArrayInput = Union[ArrowArrayExportable, BufferProtocolExportable, np.ndarray]
NameError: name 'np' is not defined

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ugh damn it 😆, well TIL, sorry for messing up your PR!

We should really have some minimal CI tests to ensure that the library can at least import without any external dependencies.

@kylebarron kylebarron enabled auto-merge (squash) October 2, 2024 21:43
@kylebarron
Copy link
Copy Markdown
Owner

This is the second error I made in the same file 😅 #207

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