feat: add DeprecationWarning for PaLM2TextEmbeddingGenerator #1018
Conversation
product-auto-label
bot
added
size: m
bigframes/deprecation.py
Outdated
|
|
||
| @functools.wraps(func1) | ||
| def new_func1(*args, **kwargs): | ||
| warnings.simplefilter("always", DeprecationWarning) |
There was a problem hiding this comment.
Please don't override people's default filters. Just use FutureWarning as recommended here: https://docs.python.org/3/library/warnings.html
See this note:
Changed in version 3.7: Previously DeprecationWarning and FutureWarning were distinguished based on whether a feature was being removed entirely or changing its behaviour. They are now distinguished based on their intended audience and the way they’re handled by the default warnings filters.
bigframes/deprecation.py
Outdated
There was a problem hiding this comment.
Can we put this in bigframes._core.deprecation? I know we haven't been the best at not polluting the bigframes.* namespace with private stuff, but since this isn't an API intended to be used by end users, I'd like to keep it clear that it's "private" to our package.
There was a problem hiding this comment.
removed the file, using typing_extension instead.
|
Consider using https://typing-extensions.readthedocs.io/en/latest/#typing_extensions.deprecated instead, as that also works well with type checkers such as |
product-auto-label
bot
added
size: s
Done |
| with pytest.warns(exceptions.ApiDeprecationWarning): | ||
| try: | ||
| llm.PaLM2TextEmbeddingGenerator() | ||
| except (Exception): |
There was a problem hiding this comment.
It feels like we are also ignoring all the exceptions? I wonder if we could just let it be raised and fail this test, if such a case occurs.
There was a problem hiding this comment.
The backend is already unavailable now. The exception is expected. We only want to check the warnings.
No description provided.