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

bug: PandasDataFrame.from_sample fails internal assertion in from_http_request #3319

Closed
mqk opened this issue Dec 6, 2022 · 5 comments · Fixed by #3320
Closed

bug: PandasDataFrame.from_sample fails internal assertion in from_http_request #3319

mqk opened this issue Dec 6, 2022 · 5 comments · Fixed by #3320
Assignees
Labels
bug Something isn't working

Comments

@mqk
Copy link
Contributor

mqk commented Dec 6, 2022

Describe the bug

I have a bento with an endpoint that takes a PandasDataFrame. I'm using .from_sample to provide example data. Now when I actually try to make network requests against that endpoint passing JSON data I get an exception:

Traceback (most recent call last):
  File "/Users/mike.kuhlen/.virtualenvs/alyssum-bento/lib/python3.8/site-packages/bentoml/_internal/server/http_app.py", line 311, in api_func
    input_data = await api.input.from_http_request(request)
  File "/Users/mike.kuhlen/.virtualenvs/alyssum-bento/lib/python3.8/site-packages/bentoml/_internal/io_descriptors/pandas.py", line 520, in from_http_request
    assert not isinstance(self._dtype, bool)
AssertionError

The reason for this error is that (a) using .from_sample sets self._dtype = True (https://github.com/bentoml/BentoML/blob/main/src/bentoml/_internal/io_descriptors/pandas.py#L421-L422), yet (b) in from_http_request we have an assertion that self._dtype may not be of type bool (https://github.com/bentoml/BentoML/blob/main/src/bentoml/_internal/io_descriptors/pandas.py#L519-L520).

The solution would seem to be to either not default to self._dtype = True (a) or to remove the assertion (b). Happy to submit a PR, but please let me know which of these options you would prefer.

To reproduce

No response

Expected behavior

No response

Environment

Environment variable

BENTOML_DEBUG=''
BENTOML_QUIET=''
BENTOML_BUNDLE_LOCAL_BUILD=''
BENTOML_DO_NOT_TRACK=''
BENTOML_CONFIG=''
BENTOML_CONFIG_OPTIONS=''
BENTOML_PORT=''
BENTOML_HOST=''
BENTOML_API_WORKERS=''

System information

bentoml: 1.0.10
python: 3.8.14
platform: macOS-12.6-x86_64-i386-64bit
uid_gid: 502:20

pip_packages
aiohttp==3.8.3
aiosignal==1.3.1
alexandria==0.0.37
anyio==3.6.2
appdirs==1.4.4
appnope==0.1.3
asgiref==3.5.2
asttokens==2.1.0
async-timeout==4.0.2
attrs==22.1.0
awscli==1.25.81
backcall==0.2.0
bentoml==1.0.10
bleach==5.0.1
botocore==1.27.80
build==0.9.0
cattrs==22.2.0
certifi==2022.9.24
cfgv==3.3.1
charset-normalizer==2.1.1
circus==0.17.1
click==8.1.3
click-log==0.4.0
cloudpickle==2.2.0
colorama==0.4.4
commonmark==0.9.1
contextlib2==21.6.0
decisioning-schemas==1.1.51
decorator==5.1.1
deepmerge==1.1.0
Deprecated==1.2.13
distlib==0.3.6
docutils==0.16
dotty-dict==1.3.1
exceptiongroup==1.0.4
executing==1.2.0
filelock==3.8.2
frozenlist==1.3.3
fs==2.4.16
gitdb==4.0.10
GitPython==3.1.29
h11==0.14.0
identify==2.5.9
idna==3.4
importlib-metadata==5.1.0
iniconfig==1.1.1
invoke==1.7.3
ipython==8.3.0
jaraco.classes==3.2.3
jedi==0.18.1
Jinja2==3.1.2
jmespath==1.0.1
joblib==1.2.0
keyring==23.11.0
lightgbm==3.0.0
MarkupSafe==2.1.1
matplotlib-inline==0.1.6
missionlane-versioning==0.7.8
more-itertools==9.0.0
multidict==6.0.2
nodeenv==1.7.0
numpy==1.23.4
opentelemetry-api==1.13.0
opentelemetry-instrumentation==0.34b0
opentelemetry-instrumentation-aiohttp-client==0.34b0
opentelemetry-instrumentation-asgi==0.34b0
opentelemetry-sdk==1.13.0
opentelemetry-semantic-conventions==0.34b0
opentelemetry-util-http==0.34b0
packaging==21.3
pandas==1.3.5
parso==0.8.3
pathspec==0.10.2
pep517==0.13.0
pexpect==4.8.0
pickleshare==0.7.5
pip-requirements-parser==31.2.0
pip-tools==6.10.0
pkginfo==1.9.2
platformdirs==2.5.4
pluggy==1.0.0
pre-commit==2.20.0
prometheus-client==0.15.0
prompt-toolkit==3.0.32
protobuf==3.9.2
psutil==5.9.4
ptyprocess==0.7.0
pure-eval==0.2.2
pyasn1==0.4.8
Pygments==2.13.0
pynvml==11.4.1
pyparsing==3.0.9
pytest==7.2.0
python-dateutil==2.8.2
python-gitlab==2.10.1
python-json-logger==2.0.4
python-multipart==0.0.5
python-semantic-release==7.19.2
pytz==2022.6
PyYAML==5.4.1
pyzmq==24.0.1
readme-renderer==37.3
requests==2.28.1
requests-toolbelt==0.10.1
rfc3986==2.0.0
rich==12.6.0
rsa==4.7.2
s3transfer==0.6.0
schema==0.7.5
scikit-learn==0.23.2
scipy==1.9.3
semver==2.13.0
simple-di==0.1.5
six==1.16.0
smmap==5.0.0
sniffio==1.3.0
stack-data==0.6.1
starlette==0.21.0
threadpoolctl==3.1.0
toml==0.10.2
tomli==2.0.1
tomlkit==0.7.0
tornado==6.2
tqdm==4.64.1
traitlets==5.5.0
twine==3.8.0
typing_extensions==4.4.0
urllib3==1.26.12
uvicorn==0.19.0
virtualenv==20.17.1
watchfiles==0.18.1
wcwidth==0.2.5
webencodings==0.5.1
wrapt==1.14.1
yarl==1.8.1
zipp==3.11.0
@mqk mqk added the bug Something isn't working label Dec 6, 2022
@aarnphm aarnphm linked a pull request Dec 7, 2022 that will close this issue
@aarnphm
Copy link
Member

aarnphm commented Dec 7, 2022

cc @mqk

@sauyon
Copy link
Contributor

sauyon commented Dec 7, 2022

We just removed the assert in #3320, but we'll be adding proper dtype validation in another PR.

@aarnphm aarnphm reopened this Dec 7, 2022
@mqk
Copy link
Contributor Author

mqk commented Dec 7, 2022

Thank you! When adding dtype validation, it would be great to continue to honor enforce_dtype. Imo, simply providing sample data should not automatically turn that on. (This is the current status quo, just saying that I would like to maintain that.)

@aarnphm aarnphm self-assigned this Jan 20, 2023
@aarnphm
Copy link
Member

aarnphm commented Jan 20, 2023

I will open a different issue to address dtype validation.

@aarnphm
Copy link
Member

aarnphm commented Jan 20, 2023

Now track at #3462

@aarnphm aarnphm closed this as completed Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants