stitching

[Sniper2k]

Contributor Checklist:

• I have added myself to the creators.json file
• I have added a changelog entry for my contribution
• I have added/updated documentation for all user-facing changes
• I have added/updated test cases

[uellue]

Hi Oleh, thanks again for this pull request! As next step, we can work on two items:

• Update your name and e-mail address in your local git configuration: https://git-scm.com/book/en/v2/Getting-Started-First-Time-Git-Setup#_your_identity That way, the commits will be attributed to you and not "Your Name you@example.com". If you'd like to use a different address than your primary GitHub one, you can add the other address to your account so that GitHub attributes them correctly: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-your-github-user-account/adding-an-email-address-to-your-github-account
• As discussed, turn the current test into a unit test for pytest. As far as I can see, your code can easily be adapted for that. The pattern is as follows:

Filename: test_stitching.py somewhere under the tests/ top-level directory in this repo.

import numpy as np

import stitch from ptychography.stitching.stitching


def test_stitching():
    known_good = ...
    your_result = stitch(...)
    assert np.allclose(known_good, your_result)

Basically a function that will throw an exception in case the actual result is different from the known good one. Pytest can do a lot more for complex test scenarios, but I think in this case the basic pattern will do just fine. :-)

You can run it like this: pytest tests/<path>/test_stitching.py

[Sniper2k]

Hi Dieter,

I added a test file, base on your template and fixed my name & email.
I had a problem testing it with pytest.

First, it was missing utils.py file. I copied it from libertem repository

Then, it was trying to access SharedData function in libertem.web.server, which is not there.

Not sure, what I can do to make it work.

[uellue]

Hi @Sniper2k, I just merged #25 that also included changes to the testing setup which should fix the issues you experienced.

In order to apply these changes to your branch, you can rebase your branch on the current master:

• Remove the utils.py file again
• Stash or commit any local changes (git status to see modified files, git stash to stow away changes to re-apply later)
• Check out the master branch of your fork
• git pull --ff-only upstream master to update your master branch to upstream. The --ff-only makes sure that you get an error message in case the two repos have diverged.
• git push the master branch of your fork to GitHub to keep it in sync
• Check out your working branch again
• git rebase master to rebase your branch onto your updated master branch
• If applicable, git stash apply to re-apply any stashed changes.
• See if this updates fixes the issues you are experiencing
• Finally, git push --force your updates to GitHub. Just doing git push without --force will give you an error message that suggests doing git pull, which you shouldn't do in this situation. Rebasing rewrites the commit history, and pulling in the previous history would mess things up. Force pushing forces your local changes to the target repo at GitHub, which is what we want here. [As a side note: A git reset --hard is required for anybody who has that branch checked out and wants to update it after you force-pushed -- same process, just pulling in the rewritten history instead of pushing it out.]

[Sniper2k]

Hi @uellue,

Thanks for the detailed explanation!
I checked and test works and exits with success now.

[uellue]

Great to hear, looks good to me! @sk1p , @SimeonEhrig would you also like to do a code review or do you think this can be merged like that?

[sk1p]

Looks good to me! The comments on the stitch function could be put into a docstring (https://numpydoc.readthedocs.io/en/latest/format.html), but that's mostly cosmetics.

[uellue]

@Sniper2k if this is ready for merging also from your perspective, you can mark it as "ready for review", i.e. not a draft PR anymore. :-)

[uellue]

Thank you, merging! 🚀