Bump Pydantic version to V2 #242
Conversation
| @@ -144,3 +145,130 @@ | |||
| from .flags import Flags | |||
| from .user_config import UserConfig | |||
| from .version import __version__ | |||
|
|
|||
There was a problem hiding this comment.
ruff complains import but not used for all these init.py files.
There was a problem hiding this comment.
I think we should have an all variable listing all imported types: astral-sh/ruff#484
It seems like ruff should be able to fix those if ran with the --fix flag.
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't know if ruff can auto fix them once we put __all__ in __init__.py. I manual fixed these.
| os.environ["FLOW360_BETA_FEATURES"] = "1" | ||
| # Comment out to pass linter. Uncomment below to run this example | ||
| # import os | ||
| # os.environ["FLOW360_BETA_FEATURES"] = "1" |
There was a problem hiding this comment.
I really cannot come up with a good way of fixing this without introducing a new error... Ruff complains that the imports are not at root level for this one.
There was a problem hiding this comment.
Beta_features are somewhat hacky right now and I think they aren't used anywhere - in the refactor we will probably need to come up with a better solution for feature flagging - I think this could be removed for now? @maciej-flexcompute
There was a problem hiding this comment.
I think @feilin-flexcompute added this BETA tag for new volume mesher pipeline so it is used for that only.
flow360/log.py
Outdated
| file = open(fname, filemode) | ||
| except: # pylint: disable=bare-except | ||
| except (IOError, OSError): |
There was a problem hiding this comment.
What else exceptions you guys can think of?
There was a problem hiding this comment.
https://peps.python.org/pep-3151/#step-1-coalesce-exception-types I think according to the standard IOError and OSError are aliases - from what I have found OSError and its subclasses indeed should cover everything thrown by open()
There was a problem hiding this comment.
Thanks! Removed the IOError
tests/params/test_outputs.py
Outdated
| @@ -36,6 +34,11 @@ def change_test_dir(request, monkeypatch): | |||
| monkeypatch.chdir(request.fspath.dirname) | |||
|
|
|||
|
|
|||
| @pytest.fixture | |||
There was a problem hiding this comment.
This fixes the ruff complaint about array_equality_override/mock_response not used but if I pass the setup_array_equality_override to the test function then the unit test fails so this probably is just a hack instead of a fix.
Not sure what is a proper way as I thought passing the setup_array_equality_override to the test function should have worked...
There was a problem hiding this comment.
There are many similar instances.
There was a problem hiding this comment.
Seems like we are not the only ones having this issue.
The suggested solution is moving the fixture definition to conftest.py it seems.
There was a problem hiding this comment.
Changed to use pytest_plugins in conftest.py
benflexcompute
requested review from
awkrupka and
maciej-flexcompute
and removed request for
awkrupka
| @@ -445,47 +435,41 @@ def validate(vec_cls, value): | |||
| setattr(cls_obj, "__get_validators__", lambda: (yield getattr(cls_obj, "validate"))) | |||
| return cls_obj | |||
|
|
|||
| # pylint: disable=invalid-name | |||
There was a problem hiding this comment.
why ruff doesn't complain here?
| @@ -33,61 +32,61 @@ | |||
| u.dimensions.inverse_area = 1 / u.dimensions.area | |||
| u.dimensions.inverse_length = 1 / u.dimensions.length | |||
|
|
|||
| # pylint: disable=no-member | |||
|
|
|||
There was a problem hiding this comment.
why ruff doesn't complain here?
| @@ -1174,7 +1156,6 @@ def __exit__(self, exc_type, exc_val, exc_tb): | |||
| _flow360_system = {u.dimension_type.dim_name: u for u in dimensions} | |||
|
|
|||
|
|
|||
| # pylint: disable=too-many-instance-attributes | |||
There was a problem hiding this comment.
why ruff doesn't complain here, what is the ruff limit?
| @@ -104,7 +104,6 @@ def _check_consistency_ddes_volume_output(values): | |||
| return values | |||
|
|
|||
|
|
|||
| # pylint: disable=line-too-long | |||
There was a problem hiding this comment.
why ruff doesn't complain here? What is the line limit?
| @@ -365,8 +362,6 @@ def _check_bet_disks_number_of_defined_polars(disk): | |||
| return disk | |||
|
|
|||
|
|
|||
| # pylint: disable=invalid-name | |||
| # pylint: disable=too-many-arguments | |||
There was a problem hiding this comment.
why ruff doesn't complain here? What is the limit of arguments?
| @@ -124,14 +110,12 @@ class ReferenceFrameExpression(ReferenceFrameBase): | |||
| theta_radians: Optional[str] = pd.Field(alias="thetaRadians") | |||
| theta_degrees: Optional[str] = pd.Field(alias="thetaDegrees") | |||
|
|
|||
| # pylint: disable=missing-class-docstring,too-few-public-methods | |||
There was a problem hiding this comment.
why ruff doesn't complain here? Is class docstring not required?
benflexcompute
force-pushed
the
BumpPydanticToV2
branch
from
900777c
to
07dbe43
Compare
|
Copied Tidy3D's ruff settings here. |
Changed to ruff for linter, now trying autofix the flow360_params.py Fixed ruff (most) and the unit tests Fixed 3 instances of E722 Do not use bare and removed all pylint instructions Remove the restriction on minor version in Pydantic use pytest_plugins instead for fixtures ruff fix Copied tidy3D's ruff settings, pending fix Fix all ruff complaints Linter Reverted example changes as it is not required
benflexcompute
force-pushed
the
BumpPydanticToV2
branch
from
6277ad6
to
fa68482
Compare
|
Forked to #247 and pylint is prefered. |
No description provided.