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

[PY-557] E2E test for Semantic Mask conversion #761

Merged
merged 4 commits into from
Jan 9, 2024
Merged

[PY-557] E2E test for Semantic Mask conversion #761

merged 4 commits into from
Jan 9, 2024

Conversation

saurbhc
Copy link
Member

@saurbhc saurbhc commented Jan 2, 2024

Problem

No e2e-test for darwin convert semantic_mask ...

Solution

Add the parameterised semantic_mask e2e-test

Changelog

Add darwin convert e2e-test for semantic_mask format

Copy link

linear bot commented Jan 2, 2024

PY-557 Semantic Mask

@saurbhc saurbhc changed the title add semantic_mask e2e test - data and cli [PY-557] E2E test for Semantic Mask conversion Jan 2, 2024
@@ -512,7 +512,7 @@ def export(
image = Image.fromarray(mask)
image.save(outfile)

with open(output_dir / "class_mapping.csv", "w") as f:
with open(output_dir / "class_mapping.csv", "w", newline="") as f:
Copy link
Member Author

@saurbhc saurbhc Jan 3, 2024

Choose a reason for hiding this comment

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

In windows, the files generated by the convert cli command have an extra line in b/w every row, which fails the e2e test - as it fails to compare the output and expected file content, but works perfectly fine on linux/macos for some reason. Found the fix from these links:

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a difference between line endings for Windows and Linux/Posix being interpreted different by the writer (\r\n vs \n), but couldn't swear to it.

Either way, you've solved it. Nice one.

@saurbhc saurbhc self-assigned this Jan 3, 2024
@saurbhc saurbhc added the enhancement New feature or request label Jan 3, 2024
@saurbhc saurbhc marked this pull request as ready for review January 4, 2024 10:28
@saurbhc saurbhc requested a review from JBWilkie January 5, 2024 10:27
Copy link
Contributor

@owencjones owencjones left a comment

Choose a reason for hiding this comment

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

Yep, straightforward approach, like it.

@@ -512,7 +512,7 @@ def export(
image = Image.fromarray(mask)
image.save(outfile)

with open(output_dir / "class_mapping.csv", "w") as f:
with open(output_dir / "class_mapping.csv", "w", newline="") as f:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a difference between line endings for Windows and Linux/Posix being interpreted different by the writer (\r\n vs \n), but couldn't swear to it.

Either way, you've solved it. Nice one.

@saurbhc saurbhc merged commit e048d80 into master Jan 9, 2024
19 checks passed
@saurbhc saurbhc deleted the PY-557 branch January 9, 2024 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants