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

fix: install inference in REST API tests #5252

Merged
merged 7 commits into from
Jul 3, 2023
Merged

fix: install inference in REST API tests #5252

merged 7 commits into from
Jul 3, 2023

Conversation

ZanSara
Copy link
Contributor

@ZanSara ZanSara commented Jul 3, 2023

Related Issues

Proposed Changes:

  • InMemoryDocumentStore requires inference and is used in the REST API tests. This PR adds that extra to the CI.

How did you test it?

CI

Notes for the reviewer

n/a

Checklist

@ZanSara ZanSara changed the title fix: install inference in restapi tests fix: install inference in REST API tests Jul 3, 2023
@github-actions github-actions bot added topic:rest_api type:documentation Improvements on the docs labels Jul 3, 2023
Comment on lines +116 to +119
if isinstance(label.answer.offsets_in_document[0], Span):
offset_start_in_document = label.answer.offsets_in_document[0].start
else:
offset_start_in_document = label.answer.offsets_in_document[0].row
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mypy was failing, so I added this block according to what the TableCell proposal specifies:

This may be confusing to users since it is not obvious that `start` should refer to `row` and `end` should refer to `column`.

@ZanSara ZanSara marked this pull request as ready for review July 3, 2023 10:44
@ZanSara ZanSara requested a review from a team as a code owner July 3, 2023 10:44
@ZanSara ZanSara requested review from bogdankostic and removed request for a team July 3, 2023 10:44
Copy link
Contributor

@bogdankostic bogdankostic left a comment

Choose a reason for hiding this comment

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

LGTM

@ZanSara ZanSara merged commit 4b380d8 into main Jul 3, 2023
@ZanSara ZanSara deleted the restapi-tests branch July 3, 2023 13:10
@ZanSara ZanSara mentioned this pull request Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants