You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ambiguity in Documentation The added documentation in line 58 is ambiguous. It's unclear whether the function preview_csv_data should or should not be used for public datasets. Consider rephrasing for clarity.
Error Handling The error message in line 59 only specifies 'baseline' and 'report' as valid modes. Ensure this aligns with all supported modes in the CLI.
Add conditional logic to ensure preview_csv_data is only called for local datasets
The conditional logic for determining whether to preview the CSV file based on the dataset type (public or local) is unclear and potentially missing. It's important to implement a clear conditional check to ensure that preview_csv_data is called only for local datasets and not for public datasets.
-the CSV file or not if the dataset is a public dataset.+if not is_public_dataset:+ preview_csv_data(dataset_path)
Suggestion importance[1-10]: 8
Why: The suggestion addresses a potential bug by clarifying when to call preview_csv_data, which is crucial for correct functionality when handling different dataset types.
8
Maintainability
Refactor mode handling to use a dictionary for scalability and maintainability
Consider handling additional modes in the start function or providing a more generic error handling mechanism. This would make the function more flexible and maintainable, especially if more modes are to be added in the future.
Why: The suggestion improves maintainability by using a dictionary for mode handling, making the code more scalable and easier to extend with additional modes.
7
Possible issue
Verify and potentially revert the default mode to 'general' if the change was unintended
The default value for the mode argument in the start command has been changed from 'general' to 'baseline'. Ensure that this change is intentional and correctly documented, as it alters the default behavior of the command.
-@click.argument('mode', default='baseline')+@click.argument('mode', default='general') # if reverting to previous default
Suggestion importance[1-10]: 5
Why: The suggestion is valid as it prompts verification of an intentional change in default behavior, which is important for maintaining expected functionality.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Closes #188
PR Type
enhancement, bug fix
Description
preview_csv_data
function should be used for public datasets in the advisor documentation.Changes walkthrough 📝
advisor.py
Clarify dataset preview conditions in advisor documentation
mle/agents/advisor.py
preview_csv_data
for public datasets.cli.py
Update CLI default mode and error message
mle/cli.py