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

external tutorials and cleanup #30

Merged
merged 45 commits into from
Feb 12, 2021
Merged

external tutorials and cleanup #30

merged 45 commits into from
Feb 12, 2021

Conversation

giovp
Copy link
Member

@giovp giovp commented Jan 25, 2021

initial tutorial for tf, pinging @hspitzer for reference, not ready for review yet

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@giovp giovp mentioned this pull request Jan 27, 2021
@giovp
Copy link
Member Author

giovp commented Feb 8, 2021

I'll push the fixed tutorials with new imagecontainer api here @hspitzer @michalk8

@michalk8
Copy link
Collaborator

michalk8 commented Feb 8, 2021

Ok, just 2 reminders:

  • please put the tutorials in docs/source/external_tutorials - the cannot be on the top-level directory, because of nbshpinx
  • please make sure that the legends are not overlapping
  • you can specify as_array='some'_key' and it will yield just the array from that key (or as_array=['key1', 'key2], which will yield a tuple at every step)

@giovp
Copy link
Member Author

giovp commented Feb 8, 2021

wll do that!

what do you mean by this?

you can specify as_array='some'_key' and it will yield just the array from that key (or as_array=['key1', 'key2], which will yield a tuple at every step)

@michalk8
Copy link
Collaborator

michalk8 commented Feb 8, 2021

what do you mean by this?

img.generate_equal_crops(as_array=True) will yield {'layer_1': <array_1>, 'layer_2': <array_2>},
img.generate_equal_crops(as_array='layer_1') will yield <array_1>
img.generate_equal_crops(as_array=['layer_1', 'layer_2']) will yield (<array_1>, <array_2>)

@giovp
Copy link
Member Author

giovp commented Feb 9, 2021

I'm gonna merge this is not tests error. I feel Ishould do another round anyway but in another PR

@giovp giovp mentioned this pull request Feb 10, 2021
@michalk8
Copy link
Collaborator

@giovp the thumbnails were wrong, you must put this info to 'metadata' part of the cell, not inside the 'source' (i.e. where the code is) - should be fixed now (I've added one for Napari from cell 6.
I've also fixed some CSS issues with images, already in scverse/squidpy#237

@hspitzer
Copy link
Contributor

@michalk8 @giovp I'll do a review of tutorials and examples now, checking for typos etc. Could we freeze the tutorial and example text until I'm done with that? Especially important for the external tutorial notebooks, as merging them is a pain. I plan to be done by tomorrow morning.

@hspitzer
Copy link
Contributor

hspitzer commented Feb 11, 2021

@michalk8 @giovp now finished with review. Looks great (and was already great)! I fixed some spelling errors and captions. I also added a lot of citations to references.bib. There are still some left in the "Morans I" section of the H&E tutorial, but I don't know if we even have to add them. They are only referenced once.

I also added the cropping and image container example from #42 @davidsebfischer .

I could not find how to specify the ordering. At least for the images examples, I'd like to have a different one than the one we have right now.

Draft of contributing guide is in in #46

@michalk8
Copy link
Collaborator

@hspitzer thanks a lot for the review!

I could not find how to specify the ordering. At least for the images examples, I'd like to have a different one than the one we have right now.

In docs/source/{examples,tutorials}.rst, you can find the glob pattern - you have to mention each tutorial explicitly - here's an example for external tutorials:

External tutorials
------------------
This section contains tutorials showcasing how squidpy can interface with external libraries.

.. nbgallery::
    :glob:

    external_tutorials/tutorial_tf
    external_tutorials/tutorial_napari
    external_tutorials/tutorial_tangram

Re spelling errors: I ran tox -e check-docs, it still caught a few - do you want to handle this or shall I?
I already did it once (allowed wordlist is in docs/source/spelling_wordlist.txt), though external tutorials have been overwritten and need to be done again. The spellchecker is using EN_US (so visualise is a incorrect spelling), but it can be changed it to EN_UK if you want, just let me know so that I can sync it up with the main repo (will do the sync after the tutorial/examples is done).

@michalk8
Copy link
Collaborator

I am going to run the spellchecker and also on the main repo.

@michalk8
Copy link
Collaborator

I've opted for EN_US and have made spellcheck part of the linting step, it should pass now (fingers crossed).
I've disabled tox -e check-docs, since some remain are broken since we're private, most likely it will be re-enabled in the main repo only.
Going to run the checker on the main repo so that after the tutorial/examples reoder, the doc stylization can be closed.

@hspitzer
Copy link
Contributor

Thanks for dealing with the automated spellchecking! EN_US is fine and more used in software (I just have a lot of UK collabs, and am getting more and more used to the UK spelling). I will push a reordering of the tutorials now.

@giovp
Copy link
Member Author

giovp commented Feb 12, 2021

awesome, I also need to push an additional tutorial on data format. It's very small and should be able to do it by this morning, thank you both!

@hspitzer
Copy link
Contributor

@michalk8 I just saw that there is another docs/source/examples.rst in the main Squidpy repo? Do I have to add the ordering there or here in squidpy notebooks? Probably in the main repo?

@michalk8
Copy link
Collaborator

@michalk8 I just saw that there is another docs/source/examples.rst in the main Squidpy repo? Do I have to add the ordering there or here in squidpy notebooks? Probably in the main repo?

Yes, you can just add it here and I will sync it in scverse/squidpy#237

@hspitzer
Copy link
Contributor

@michalk8 I just saw that there is another docs/source/examples.rst in the main Squidpy repo? Do I have to add the ordering there or here in squidpy notebooks? Probably in the main repo?

Yes, you can just add it here and I will sync it in theislab/squidpy#237

great, thanks a lot! Already added here.

@giovp
Copy link
Member Author

giovp commented Feb 12, 2021

I should be able to submit this last tutorial thsi afternoon,and then we can merge this

@giovp
Copy link
Member Author

giovp commented Feb 12, 2021

ok, added the tutorial, I would merge this as is and ask couple of people to review it his weekedn. But can submit minor PR early next week separate for this. If tests passes: LGTM!!!!!!!!

@giovp giovp merged commit 287c1e6 into master Feb 12, 2021
@giovp giovp deleted the tf branch February 12, 2021 13:59
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.

4 participants