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

seg_suffix Requires Explicit File Extension #848

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Conversation

srivarra
Copy link
Contributor

@srivarra srivarra commented Dec 2, 2022

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Closes #842. Using file extensions other than .tiff for seg_suffix in the Notebooks: Pixel Clustering, Cell Clustering will not work due to hard-coded in extension types. It should be backwards compatible for now.

How did you implement your changes

seg_suffix used in Notebooks 2,3 will now work appropriately with post_cluster_utils.create_mantis_project and plot_utils.create_mantis_dir.

  • seg_suffix now requires the file extension in addition to the suffix.
  • Added the constant EXTENSION_TYPES to ark.settings.py which contains file extensions for images, archival formats, and data formats.

Remaining issues

N/A.

@srivarra srivarra added the bug Something isn't working label Dec 2, 2022
@srivarra srivarra self-assigned this Dec 2, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@srivarra srivarra marked this pull request as ready for review December 5, 2022 19:41
@ngreenwald
Copy link
Member

These lines are triggering a change in coveralls on different PRs. In theory we shouldn't ever be generating corrupted feather files, which is why they're variably covered. Can you either exclude them from being counted in coveralls, or generate a corrupted feather file so that the coverage is consistent?
image

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks good, see above

@srivarra
Copy link
Contributor Author

srivarra commented Dec 6, 2022

These lines are triggering a change in coveralls on different PRs. In theory we shouldn't ever be generating corrupted feather files, which is why they're variably covered. Can you either exclude them from being counted in coveralls, or generate a corrupted feather file so that the coverage is consistent?

Got it to ignore the (ArrowInvalid, OSError, IOError) lines.

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks good

@ngreenwald ngreenwald merged commit 7f6d561 into main Dec 6, 2022
@ngreenwald ngreenwald deleted the mantis/img_suffix branch December 6, 2022 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent seg_suffix causing error in pixel clustering notebook
2 participants