Skip to content

Conversation

xusliebana
Copy link
Contributor

Add an interface to use multiple input annotations columns

Description

Motivation and Context

We need to have an interface to allow to use of multiple input annotations in the healthcare repo.

How Has This Been Tested?

I created a unit test that show that is working that approach

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Code improvements with no or little impact
  • [X ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ X] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING page.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@maziyarpanahi maziyarpanahi self-requested a review November 10, 2021 11:41
@maziyarpanahi maziyarpanahi self-assigned this Nov 10, 2021
@maziyarpanahi maziyarpanahi added new-feature Introducing a new feature on-hold cannot be merged right away labels Nov 10, 2021
@xusliebana xusliebana force-pushed the multicolumns-approach branch from f6c2a49 to 2aa0d3a Compare November 10, 2021 15:50
@xusliebana
Copy link
Contributor Author

@maziyarpanahi I add it the documentation and remove one commit that was from other branch

@maziyarpanahi
Copy link
Contributor

@xusliebana the references to CHUNK in test should change to DOCUMENT.

@xusliebana xusliebana force-pushed the multicolumns-approach branch 2 times, most recently from 3770b2e to d190197 Compare November 10, 2021 16:38
@xusliebana xusliebana force-pushed the multicolumns-approach branch from d190197 to 3f52364 Compare November 10, 2021 16:41
@xusliebana
Copy link
Contributor Author

@maziyarpanahi Done

@maziyarpanahi
Copy link
Contributor

Thanks @xusliebana
This will be in the next release of spark-nlp==3.3.3

@maziyarpanahi maziyarpanahi added the DON'T MERGE Do not merge this PR label Nov 13, 2021
@maziyarpanahi maziyarpanahi changed the base branch from master to release/333-release-candidate November 17, 2021 10:10
@maziyarpanahi maziyarpanahi removed DON'T MERGE Do not merge this PR on-hold cannot be merged right away labels Nov 17, 2021
@maziyarpanahi maziyarpanahi merged commit c4f338d into release/333-release-candidate Nov 17, 2021
/**
* Trait used to create annotators with input columns of variable length.
* */
trait HasMultipleInputAnnotationCols extends HasInputAnnotationCols {
Copy link
Contributor

Choose a reason for hiding this comment

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

@maziyarpanahi @xusliebana , don't you guys think that the name for this trait could be HasVariableCountInputAnnotationCols

HasInputAnnotationCols already supports multiple input columns, the main change I see here is that the number of columns will no longer be defined in a 'declarative' fashion inside the library, but in a more dynamic way by the user, during annotator setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

@albertoandreottiATgmail Yes, that totally makes sense! I'll leave it to you and @xusliebana to rename this and the unit tests. (@xusliebana you can directly fix the changes inside release/333-release-candidate branch)

@albertoandreottiATgmail Could you please also review removing final from final def setInputCols()? I am not sure why we used final so I want to be sure removing it won't be risky in some scenarios.

@KshitizGIT KshitizGIT deleted the multicolumns-approach branch March 2, 2023 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature Introducing a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants