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

[ez] Autoformatting #1115

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

[ez] Autoformatting #1115

wants to merge 2 commits into from

Conversation

rossdanlm
Copy link
Contributor

@rossdanlm rossdanlm commented Feb 2, 2024

[ez] Autoformatting

Ran fd --glob '*.py' python/src/aiconfig/ | xargs python -m 'scripts.lint' --mode=fix --files


Stack created with Sapling. Best reviewed with ReviewStack.

Copy link
Contributor

@rholinshead rholinshead left a comment

Choose a reason for hiding this comment

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

Is there some extension or something we each need to have set up locally to make sure these formatting changes happen on save? Not sure why we need to keep manually running the formatter

Rossdan Craig rossdan@lastmileai.dev added 2 commits February 2, 2024 19:45
Tanya noticed a bug when we accidentally added "field " with a space instead of "field", so trimming the value.

I originally tried to fix this in the editor config itself in `SettingsPropertyRenderer`, but that was complicated since it meant that this would reset the string value state for the string, and you couldn't keep typing empty spaces at the begininng or end of strings otherwise it would keep trimming them. Therefore, I decided to make this change in all places where we set the model names or fields for main SDK. This may seem a bit controversial (ex: you now can't have two separate models called "  my_model  " vs. "my_model" but I feel this is fine

Note: Because we could have existing configs that have model names already, I only changed the setter methods, not the getter ones like `get_model()`, so this is not a breaking change

## Test Plan
```
aiconfig_path=./cookbooks/Gradio/huggingface.aiconfig.json
parsers_path=./cookbooks/Gradio/aiconfig_model_registry.py
aiconfig edit --aiconfig-path=$aiconfig_path --server-port=8080 --server-mode=debug_servers --parsers-module-path=$parsers_path
```
It now works with leading or trailing whitespace

https://github.com/lastmile-ai/aiconfig/assets/151060367/116b4ef4-b1f9-4a15-9d07-e1fb9d995245
Ran `fd --glob '*.py' python/src/aiconfig/ | xargs python -m 'scripts.lint' --mode=fix --files`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants