Refactor optional import logic and verify minimum versions #3160
Conversation
Set minimum vegafusion version to 1.4.0 and minimum vl_convert version to 0.12.0.
| ) from err | ||
|
|
||
|
|
||
| def import_pyarrow_interchange() -> ModuleType: |
There was a problem hiding this comment.
For consistency with the others, I updated this to use pa.__version__ directly rather than importlib.metadata.version. Does anyone recall whether there was a particular reason to use importlib?
There was a problem hiding this comment.
__version__ is not a required property of a Python package, I think it could for example be defined only in pyproject.toml without exposing __version__. I've also recently seen a deprecation warning of a package when accessing __version__. importlib.metadata.version is a more general and robust way to retrieve the version information based on the metadata of a package.
Although __version__ works for all these packages, I'd slightly prefer importlib. What do you think?
There was a problem hiding this comment.
These are good points, I'll look at switching over to importlib.
| import_pyarrow_interchange() | ||
| return True | ||
| except ImportError: | ||
| return False |
There was a problem hiding this comment.
moved to util._importers
| vega_spec, | ||
| dataset_names, | ||
| get_local_tz(), |
There was a problem hiding this comment.
This is now the default value of the local_tz argument in VegaFusion 1.4.0.
altair/utils/_importers.py
Outdated
|
|
||
|
|
||
| def import_vl_convert() -> ModuleType: | ||
| min_vlc_version = "0.12.0" |
There was a problem hiding this comment.
Is this version number determined by what Vega-Lite version is bundled in VLC? If yes, could you add a bullet point to the Updating the Vega-Lite version section in NOTES_FOR_MAINTAINERS.md to update this version number every time we update Vega-Lite?
| ) from err | ||
|
|
||
|
|
||
| def import_pyarrow_interchange() -> ModuleType: |
There was a problem hiding this comment.
__version__ is not a required property of a Python package, I think it could for example be defined only in pyproject.toml without exposing __version__. I've also recently seen a deprecation warning of a package when accessing __version__. importlib.metadata.version is a more general and robust way to retrieve the version information based on the metadata of a package.
Although __version__ works for all these packages, I'd slightly prefer importlib. What do you think?
|
could you glance over this once more @binste? I switched to |
|
Looks good, thanks @jonmmease! :) |
This PR moves the optional import logic for pyarrow, vegafusion, and vl-convert to
util._importersand checks for minimum supported versions of vegafusion (1.4.0) and vl-convert (0.12.0).