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

[ENH] Implemented getter to create JSON representation of continuous column #430

Merged
merged 9 commits into from
May 23, 2023
Merged

[ENH] Implemented getter to create JSON representation of continuous column #430

merged 9 commits into from
May 23, 2023

Conversation

rmanaem
Copy link
Contributor

@rmanaem rmanaem commented May 19, 2023

Closes #407

Changes proposed in this pull request:

  • Modified transformationHeuristics according to this comment
  • Updated getTransformOptions getter
  • Updated getTransformOptions getter's unit test
  • Implemented getContinuousJsonOutput getter
  • Implemented unit test for getContinuousJsonOutput getter
  • Removed unnecessary test cases from getCategoricalJsonOutput getter's unit test

Checklist

  • PR has an interpretable title with a prefix ([ENH], [BUG], [DOC], [INFRA], [MAINT])
  • PR links to Github issue with mention Closes #XXXX
  • Tests pass
  • Code is properly formatted

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions

@rmanaem rmanaem linked an issue May 19, 2023 that may be closed by this pull request
getter to create JSON representation of continuous column #407
Closed
3 tasks
@jarmoza jarmoza self-requested a review May 19, 2023 16:13
jarmoza
jarmoza suggested changes May 19, 2023
View reviewed changes
Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

@rmanaem Aside from the suggestion for alphabetizing object keys, I think the only major change to consider is why the levels and missing values test were removed from the categorical options getter tests in this PR.

cypress/unit/store-getter-getContinousJsonOutput.cy.js Show resolved Hide resolved
cypress/unit/store-getter-getContinousJsonOutput.cy.js Outdated Show resolved Hide resolved
cypress/unit/store-getter-getTransformOptions.cy.js Outdated Show resolved Hide resolved
store/index.js Outdated Show resolved Hide resolved
cypress/unit/store-getter-getCategoricalJsonOutput.cy.js Show resolved Hide resolved
@rmanaem rmanaem requested a review from jarmoza May 23, 2023 15:18
@rmanaem rmanaem self-assigned this May 23, 2023
jarmoza
jarmoza approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

🧑‍🍳

@rmanaem rmanaem merged commit cb9cb70 into dev_components_talk_to_store_directly May 23, 2023
@rmanaem rmanaem deleted the feat-407 branch May 23, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jarmoza jarmoza jarmoza approved these changes

Assignees

@rmanaem rmanaem

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

getter to create JSON representation of continuous column
2 participants
@rmanaem @jarmoza