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

32 update pipeline metro map on the readme page #36

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

khersameesh24
Copy link
Collaborator

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs- [ ] If necessary, also make a PR on the nf-core/spatialxe branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

Copy link
Collaborator

Choose a reason for hiding this comment

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

please also provide an svg version. makes it easier to edit it in the future.

Looks very nice, some small nit-picking:

  • The orange and the green are the same kind of brown for red-green-sight-deficiant people.
  • try to center seggar and resegment nodes inbetwen the curves (also resegment could need some padding to the left)
  • you have enough space for cellpase and proseq to be actually next to their node.
  • try to make the gray boxes have the same distance to the green line after baysor
  • I would try to make the relabel process in the same hight as 5the coordinates/mask one, makes it easier to see that it is actually another input path.
  • speaking of input: their symbol is not expalined in the legend
  • I don't know if the arrows are actually needed. try if everything is still readable without them.
  • the folder for xenium bundle is not centered, smae with the ro-crate symbol to the right and the spatialdata icon to the left of coordinate mask. similarly, the gen epanel json icon and the html qc reports are not centeredd horizontally. and the first coorcinates/mask is neither.
  • thinking about the color usage. what if instead of approach the encode the original data. so you could use blue for transcripts and orange for morphology and drop green and just combine the two after the baysor step and thereafter

but again this are just small things the whole thing looks very clear, congrats!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @mashehu, I have made the suggested changes and updated the metromap. Also, added the svg for the metromap.

@heylf heylf mentioned this pull request Nov 27, 2024
9 tasks
@heylf
Copy link
Collaborator

heylf commented Nov 29, 2024

@khersameesh24 one tiny remark that I think should be changed is the input of the relabeling what mashehu already mentioned. It is not clear that the json file for the relabeling is an input. Can you just add another arrow there please?

Or for the version without arrow, put the json file above and not on the same height at the output files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update pipeline metro map on the README page
3 participants