-
Notifications
You must be signed in to change notification settings - Fork 6
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
Define table specifications #582
Define table specifications #582
Conversation
Coverage reportThe coverage rate went from None of the new lines are part of the tested code. Therefore, there is no coverage data about them. |
@jluethi a first draft of the new docs page is taking form. Any high-level feedback on the layout and contents is already welcome, while I'd suggest that for a more detailed review you can directly work on the source code in this branch. Here are examples of how to serve the documentation locally, to have the new page visible at http://localhost:8001/roi_tables: git checkout 529-write-docs-page-for-fractal-tasks-core-rois-v1
# With poetry
poetry install --with docs --all-extras
poetry run mkdocs serve --config-file mkdocs.yml --dev-addr localhost:8001
# Without poetry
python -m venv myenv
source myenv/bin/activate
pip install -r docs/doc-requirements.txt
mkdocs serve --config-file mkdocs.yml --dev-addr localhost:8001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great first version @tcompa
Let's discuss at our call. A few bigger points:
Is it the doc for ROI tables or tables in general?
Title is ROI, but text says general table spec. Let's briefly discuss at our call
In short, I see 2 equivalent use-cases:
- ROIs (and sub-cases): Each row is a region of interest
- Measurements: Each row is a measured object (e.g. related to a given label in the label image)
If our z_micrometer are not used in micrometers, but as arbitrary units: That's actually a bug. We just never noticed, because many users image with z pixel size of 1. But some users image with 0.6 or 2.0
Outlook: Axis generalization |
Link to PR for fusing FOVs to single well: Nomenclature: |
TODO: Mention masked loading functionality |
Fractal core Tables (subset of proposed table spec) 3 tables: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, I really like the reworked version @tcompa . Great job!
Only open remaining question is about wording on whether Z is used for well & FOV ROIS, let's briefly discuss tomorrow. The rest are minor wording suggestions or typos :)
Thanks for the review. |
Close #529
Close #587
Close #588
Checklist before merging
CHANGELOG.md