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(study): normalize study path #2316

Merged
merged 40 commits into from
Feb 3, 2025
Merged

Conversation

smailio
Copy link
Contributor

@smailio smailio commented Jan 27, 2025

The front end can't handle windows path, the desktop version on windows won't display the study hierarchy correctly.

The front end relies on the folder field of Study. This PR adds a validator to that field that normalize any path, including windows path to a posix path.

@pull-request-size pull-request-size bot added size/M and removed size/L labels Jan 27, 2025
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jan 27, 2025
@pull-request-size pull-request-size bot added size/M and removed size/L labels Jan 29, 2025
Anis SMAIL added 2 commits January 29, 2025 16:43
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jan 29, 2025
@@ -66,9 +66,6 @@ def test_get(tmp_path: str, project_path) -> None:
data = {"titi": 43}
sub_route = "settings"

path = path_study / "settings"
key = "titi"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These variables were unused

@@ -100,7 +97,6 @@ def test_get_cache(tmp_path: str) -> None:
path_study.mkdir()
(path_study / "settings").mkdir()
(path_study / "study.antares").touch()
path = path_study / "settings"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was unused

@MartinBelthle MartinBelthle changed the title feature(study): normalize study path feat(study): normalize study path Jan 30, 2025
Copy link
Contributor

@MartinBelthle MartinBelthle left a comment

Choose a reason for hiding this comment

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

Before merging we should generate a Desktop version to give to Alexander for him to try it

antarest/study/model.py Outdated Show resolved Hide resolved
antarest/study/model.py Outdated Show resolved Hide resolved
@@ -288,6 +289,23 @@ def __eq__(self, other: t.Any) -> bool:
def to_json_summary(self) -> t.Any:
return {"id": self.id, "name": self.name}

@validates("folder") # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sqlachemu doesn't provide a type for this decorator

if not path:
return path
pure_path = PurePath(path)
return pure_path.as_posix()
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: all this could be simplfied in one line:
return PurePath(path).as_posix() if path else path

@MartinBelthle MartinBelthle merged commit b85fe72 into dev Feb 3, 2025
13 of 14 checks passed
@MartinBelthle MartinBelthle deleted the feature/2547-normalize-study-path branch February 3, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants