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.

Sign up for GitHub

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

Jump to bottom

426 bug msg keyerror data #428

Merged
14 commits merged into from
Nov 9, 2022
Merged

426 bug msg keyerror data #428

14 commits merged into from
Nov 9, 2022

Conversation

bsuryadevara
Copy link
Contributor

  • Users can pass the name of a column that has to be pre-processed more easily with parameterized pre-process columns.

@bsuryadevara bsuryadevara added bug Something isn't working non-breaking Non-breaking change 2 - In Progress labels Nov 1, 2022
@bsuryadevara bsuryadevara self-assigned this Nov 1, 2022
@bsuryadevara bsuryadevara marked this pull request as ready for review November 1, 2022 19:48
@bsuryadevara bsuryadevara requested review from a team as code owners November 1, 2022 19:48
@bsuryadevara bsuryadevara added 3 - Ready for Review ! - Hotfix Related to a Hotfix Release and removed 2 - In Progress labels Nov 1, 2022
@bsuryadevara bsuryadevara linked an issue Nov 1, 2022 that may be closed by this pull request
[BUG]: current runnable. Exception msg: KeyError: 'data' #426
Closed
2 tasks
@cwharris
Copy link
Contributor

cwharris commented Nov 2, 2022

rerun tests

mdemoret-nv
mdemoret-nv requested changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

We need a test case to exercise this new functionality.

morpheus/stages/preprocess/preprocess_nlp_stage.py Show resolved Hide resolved
dagardner-nv
dagardner-nv approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@dagardner-nv dagardner-nv left a comment

Choose a reason for hiding this comment

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

LGTM

minor nitpick to make one of the arguments a reference, but looks good in general.

morpheus/_lib/include/morpheus/stages/preprocess_nlp.hpp Outdated Show resolved Hide resolved
mdemoret-nv
mdemoret-nv requested changes Nov 3, 2022
View reviewed changes
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

Need to have the column argument come after stride otherwise this is a breaking change.

morpheus/_lib/include/morpheus/stages/preprocess_nlp.hpp Outdated Show resolved Hide resolved
@mdemoret-nv mdemoret-nv changed the base branch from branch-22.11 to branch-22.09 November 3, 2022 22:14
@mdemoret-nv mdemoret-nv requested a review from a team as a code owner November 3, 2022 22:14
@mdemoret-nv mdemoret-nv changed the base branch from branch-22.09 to branch-22.11 November 3, 2022 22:24
@mdemoret-nv mdemoret-nv changed the base branch from branch-22.11 to branch-22.09 November 3, 2022 22:43
@mdemoret-nv
Copy link
Contributor

rerun tests

@dagardner-nv
Copy link
Contributor

\rerun tests

@dagardner-nv
Copy link
Contributor

/rerun tests

mdemoret-nv
mdemoret-nv approved these changes Nov 7, 2022
View reviewed changes
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

New tests look good. CI has been verified to complete locally without any issues.

@mdemoret-nv
Copy link
Contributor

@gpucibot merge

@ghost ghost merged commit 521dd9f into nv-morpheus:branch-22.09 Nov 9, 2022
@bsuryadevara bsuryadevara deleted the 426-bug-msg-keyerror-data branch May 5, 2023 17:50
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwharris cwharris cwharris approved these changes

@raykallen raykallen raykallen approved these changes

@mdemoret-nv mdemoret-nv mdemoret-nv approved these changes

@dagardner-nv dagardner-nv dagardner-nv approved these changes

Assignees

@bsuryadevara bsuryadevara

Labels
! - Hotfix Related to a Hotfix Release bug Something isn't working non-breaking Non-breaking change
Projects
Morpheus Boards
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: current runnable. Exception msg: KeyError: 'data'
5 participants
@bsuryadevara @cwharris @mdemoret-nv @dagardner-nv @raykallen