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

Feature/markdown class #276

Merged
merged 21 commits into from
Aug 2, 2024
Merged

Feature/markdown class #276

merged 21 commits into from
Aug 2, 2024

Conversation

rugeli
Copy link
Collaborator

@rugeli rugeli commented Jul 22, 2024

Problem

What is the problem this work solves, including
closes #135

Solution

What I/we did to solve this problem

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Steps to Verify:

  1. we are expecting all existing functionality works the same after switching to the MarkdownWriter
  2. verify that github bot comments in this PR are generated correctly
  3. (optional) run test cases: pytest cellpack/tests/test_markdown_writer.py

@rugeli rugeli force-pushed the feature/markdown-class branch 2 times, most recently from 0ff5204 to bcab5b4 Compare July 23, 2024 01:58
Copy link

github-actions bot commented Jul 26, 2024

Packing analysis report

Analysis for packing results located at cellpack/tests/outputs/test_spheres/spheresSST

Ingredient name Encapsulating radius Average number packed
ext_A 25 236.0

Packing image

Packing image

Distance analysis

Expected minimum distance: 50.00
Actual minimum distance: 50.01

Ingredient key Pairwise distance distribution
ext_A Distance distribution ext_A

@rugeli
Copy link
Collaborator Author

rugeli commented Jul 26, 2024

Note: This is a WIP. I'll request reviews once the images and markdown file formatting are adjusted.

@rugeli rugeli requested review from meganrm and mogres July 29, 2024 21:48
Copy link
Collaborator

@mogres mogres left a comment

Choose a reason for hiding this comment

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

Very nice updates! The output report in the PR comment also looks accurate.

@@ -431,3 +435,108 @@ def save(
self.save_as_simularium(env, seed_to_results_map)
else:
print("format output " + output_format + " not recognized (json,python)")

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want this in __init__.py? I feel like this could be moved to a separate file MarkdownWriter.py in the same folder

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, fixed!


with open(report_path, "r") as f:
report = f.read()
assert "Test Image" in report
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very cool!

Copy link
Member

@meganrm meganrm left a comment

Choose a reason for hiding this comment

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

this looks great!

@rugeli rugeli merged commit 339e399 into main Aug 2, 2024
7 checks passed
@rugeli rugeli deleted the feature/markdown-class branch August 2, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create writer class for markdown files
3 participants