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

feat(SemanticLayerSchema): making column type optional and avoid dumping and parsing null values in the SemanticLayerSchema #1521

Merged
merged 11 commits into from
Jan 15, 2025

Conversation

scaliseraoul
Copy link
Contributor

@scaliseraoul scaliseraoul commented Jan 15, 2025


Important

Make column type optional in SemanticLayerSchema and avoid dumping/parsing null values, with related documentation and code updates.

  • Behavior:
    • type in Column class in semantic_layer_schema.py is now optional.
    • params in Transformation class in semantic_layer_schema.py is now optional.
    • to_dict() method in SemanticLayerSchema now excludes None values.
    • to_yaml() method in SemanticLayerSchema uses to_dict() to exclude None values.
  • Documentation:
    • Updated docs/v3/semantic-layer.mdx to reflect changes in column definition from dict to list.
    • Updated docs/v3/contributing.mdx to change make tests to make test_all.
  • Code Improvements:
    • Changed _get_abs_dataset_path() in loader.py to return a string path.
    • Updated _create_yml_template() in base.py to handle optional columns.
    • Updated test_save_creates_correct_schema() in test_dataframe.py to reflect schema changes.

This description was created by Ellipsis for f889c86. It will automatically update as commits are pushed.

scaliseraoul and others added 11 commits January 13, 2025 15:32
…e, removes unreachable code and adds tests for _load_from_local_source
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
…into release/v3

* 'release/v3' of https://github.com/scaliseraoul/pandas-ai:
  refactor(loader): renames _load_from_source to _load_from_local_source removes unreachable code and adds tests for _load_from_local_source (sinaptik-ai#1514)
…ing and parsing null values in the SemanticLayerSchema
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jan 15, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to f889c86 in 1 minute and 38 seconds

More details
  • Looked at 254 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. docs/v3/contributing.mdx:67
  • Draft comment:
    Ensure that the change from make tests to make test_all is reflected in the actual Makefile or test script to maintain consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change in the test command from make tests to make test_all in the documentation should be reflected in the actual Makefile or test script. This ensures consistency between the documentation and the actual commands used.
2. pandasai/dataframe/base.py:179
  • Draft comment:
    Check if columns_dict is not None before mapping it to Column objects to avoid potential TypeError.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
3. pandasai/data_loader/semantic_layer_schema.py:38
  • Draft comment:
    Ensure that params is checked for None before accessing its elements to avoid potential AttributeError.
  • Reason this comment was not posted:
    Comment did not seem useful.
4. pandasai/data_loader/semantic_layer_schema.py:27
  • Draft comment:
    Check if type is not None before validating it to avoid potential TypeError.
  • Reason this comment was not posted:
    Marked as duplicate.
5. pandasai/data_loader/semantic_layer_schema.py:30
  • Draft comment:
    Consider rephrasing the error message for unsupported column type for clarity:
                f"Unsupported column type: {type}. Supported types are: {', '.join(VALID_COLUMN_TYPES)}."
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_8URjNb3RqkoKGaja


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@gventuri gventuri merged commit 26fc930 into sinaptik-ai:release/v3 Jan 15, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants