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

Vertical col support #945

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Vertical col support #945

wants to merge 10 commits into from
+271 −72

Conversation

Nitish1814
Copy link
Contributor

No description provided.

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 8, 2024
View reviewed changes
common/client/src/main/java/zingg/common/client/ZFrame.java Fixed Show fixed Hide fixed
common/client/src/main/java/zingg/common/client/util/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
common/client/src/main/java/zingg/common/client/util/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
common/client/src/main/java/zingg/common/client/util/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
common/core/src/main/java/zingg/common/core/executor/LabelUpdater.java
@@ -124,7 +124,7 @@
return updatedRecords;
}

protected int getUserInput(ZFrame<D,R,C> lines,ZFrame<D,R,C> currentPair,String cluster_id) {
protected int getUserInput(ZFrame<D,R,C> lines,ZFrame<D,R,C> currentPair,String cluster_id) throws ZinggClientException {

Check warning

Code scanning / PMD

The method parameter name 'z_source' doesn't match '[a-z][a-zA-Z0-9]*' Warning

The method parameter name 'cluster_id' doesn't match '[a-z][a-zA-Z0-9]*'
spark/client/src/main/java/zingg/spark/client/util/SparkVerticalDisplayUtility.java Fixed Show fixed Hide fixed
spark/client/src/main/java/zingg/spark/client/util/SparkVerticalDisplayUtility.java Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 11, 2024
View reviewed changes
common/client/src/main/java/zingg/common/client/util/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
common/client/src/main/java/zingg/common/client/util/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
common/core/src/test/java/zingg/common/core/util/TestVerticalDisplayUtility.java
public final String dob;
public final String row_uuid;

public Customer(String fname, String lname, String ssn, String dob, String row_uuid) {

Check warning

Code scanning / PMD

The method parameter name 'z_source' doesn't match '[a-z][a-zA-Z0-9]*' Warning test

The method parameter name 'row_uuid' doesn't match '[a-z][a-zA-Z0-9]*'
spark/client/src/main/java/zingg/spark/client/util/SparkVerticalDisplayUtility.java Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 12, 2024
View reviewed changes
common/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayModel.java
@@ -0,0 +1,4 @@
package zingg.common.client.util.verticalDisplay;

Check warning

Code scanning / PMD

Package name contains upper case characters Warning

Package name contains upper case characters
Copy link
Member

Choose a reason for hiding this comment

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

As per Java conventions, package names should not contain upper case. We made that mistake earlier but it makes sense to follow the conventions.

common/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayModel.java
@@ -0,0 +1,4 @@
package zingg.common.client.util.verticalDisplay;

public abstract class VerticalDisplayModel {

Check warning

Code scanning / PMD

No abstract method which means that the keyword is most likely used to prevent instantiation. Use a private or protected constructor instead. Warning

No abstract method which means that the keyword is most likely used to prevent instantiation. Use a private or protected constructor instead.
Copy link
Member

Choose a reason for hiding this comment

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

why not make it an interface /functional interface?

...lient/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayTwoRowModel.java
@@ -0,0 +1,13 @@
package zingg.common.client.util.verticalDisplay;

Check warning

Code scanning / PMD

Package name contains upper case characters Warning

Package name contains upper case characters
...lient/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayTwoRowModel.java
package zingg.common.client.util.verticalDisplay;

public class VerticalDisplayTwoRowModel extends VerticalDisplayModel {
private final String Field;

Check warning

Code scanning / PMD

Avoid unused private fields such as 'session'. Warning

Avoid unused private fields such as 'Field'.
...lient/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayTwoRowModel.java

public class VerticalDisplayTwoRowModel extends VerticalDisplayModel {
private final String Field;
private final String Value1;

Check warning

Code scanning / PMD

Avoid unused private fields such as 'session'. Warning

Avoid unused private fields such as 'Value1'.
...lient/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayTwoRowModel.java
private final String Value1;
private final String Value2;

public VerticalDisplayTwoRowModel(String Field, String Value1, String Value2) {

Check warning

Code scanning / PMD

The method parameter name 'z_source' doesn't match '[a-z][a-zA-Z0-9]*' Warning

The method parameter name 'Value2' doesn't match '[a-z][a-zA-Z0-9]*'
...on/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayUtility.java
@@ -0,0 +1,45 @@
package zingg.common.client.util.verticalDisplay;

Check warning

Code scanning / PMD

Package name contains upper case characters Warning

Package name contains upper case characters
...on/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
...on/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayUtility.java
List<VerticalDisplayModel> rowList = getList(comparison_pairs, columns);
return dfObjectUtil.getDFFromObjectList(rowList, VerticalDisplayTwoRowModel.class);
} catch (Exception exception) {
throw new ZinggClientException("Error occurred while converting to vertical, " + exception.getMessage());

Check warning

Code scanning / PMD

Thrown exception does not preserve the stack trace of exception 't' on all code paths Warning

Thrown exception does not preserve the stack trace of exception 'exception' on all code paths
...on/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayUtility.java Fixed Show fixed Hide fixed
sonalgoyal
sonalgoyal requested changes Nov 12, 2024
View reviewed changes
common/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayModel.java
@@ -0,0 +1,4 @@
package zingg.common.client.util.verticalDisplay;
Copy link
Member

Choose a reason for hiding this comment

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

As per Java conventions, package names should not contain upper case. We made that mistake earlier but it makes sense to follow the conventions.

common/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayModel.java
@@ -0,0 +1,4 @@
package zingg.common.client.util.verticalDisplay;

public abstract class VerticalDisplayModel {
Copy link
Member

Choose a reason for hiding this comment

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

why not make it an interface /functional interface?

...on/client/src/main/java/zingg/common/client/util/verticalDisplay/VerticalDisplayUtility.java Fixed Show resolved Hide resolved
common/core/src/test/java/zingg/common/core/util/TestVerticalDisplayUtility.java
}

@Test
public void testWithoutNulls() throws Exception, ZinggClientException {
Copy link
Member

Choose a reason for hiding this comment

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

can you also try with an empty dataframe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonalgoyal sonalgoyal sonalgoyal requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Nitish1814 @sonalgoyal