-
Notifications
You must be signed in to change notification settings - Fork 1
Add development dependencies #5
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
base: main
Are you sure you want to change the base?
Conversation
| parameters=[ | ||
| ["Name"], | ||
| ["Value"], | ||
| ], # Do not change the column names of the parameters table! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the parameters argument formatting, do we need to break it down into multiple lines? This was automatically formatted, but in case of tables with few columns, I'm concerned that we'll unnecessarily create new lines. Maybe there's a way to configure ruff to only break into multiple lines when the number of characters in the line is above, say, 120 or something. @luizkubaski do you know if that's possible? I remember you played with ruff as well
| input_schema.add_parameter('Sample Int Parameter', default_value=1, | ||
| **non_negative_float(max=10, inclusive_max=True)) | ||
| input_schema.add_parameter('Sample Date Parameter', default_value='11/21/2022', datetime=True) | ||
| input_schema.add_parameter("Sample Text Parameter", default_value="Any Text", **text()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I didn't get the ruff pattern, like for Sample Text Parameter it kept a single line, but for Sample Date Parameter below it added more lines, although it also fits in a single line with just a few characters long
| old_sum = self.dat.sample_input_table["Data Field Two"].sum() | ||
| dat = mip_template.data_prep_solve(self.dat) | ||
| new_sum = dat.sample_input_table['Data Field Two'].sum() | ||
| close_enough = isclose(new_sum, self.params['Sample Float Parameter'] * old_sum, rel_tol=1e-2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again about ruff behavior, for close_enough it used all arguments in a new line, but below we see
self.assertSetEqual(
set(sln.sample_output_table["Data Field"]),
{"Option 1", "Option 2"},
"Main solve check",
)one argument per line
| sample_output_table_df = sample_input_table_df[['Primary Key Two', 'Data Field Two']] | ||
| if params["Sample Two Values Parameter"] == "Value 1": | ||
| sample_output_table_df = sample_input_table_df[ | ||
| ["Primary Key One", "Data Field One"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, it seems unnecessary to have these additional lines
| 'License :: OSI Approved :: MIT License', | ||
| 'Operating System :: OS Independent' | ||
| ] | ||
| dependencies = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want jupyterlab and ruff to be development dependencies, right? Otherwise, when uploading to Mip Hub for instance, it would also install ruff although it's unnecessary for deployment.
You can do that by running
uv remove jupyterlab ruff
uv add --dev jupyterlab ruffThis creates a new table in the end of pyproject.toml like
[dependency-groups]
dev = [
"jupyterlab",
"ruff",
]
Fortunately, uv sync by default synchronizes dependencies from the main dependencies table, and also the development one. uv remove <package> and uv add <package> manage the main dependencies only; for development ones, use --dev flag.
ruff formatruff format path/to/file