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

tests(disordered_tracing): Bulks out missing unittests #978

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

ns-rse
Copy link
Collaborator

@ns-rse ns-rse commented Oct 22, 2024

Making a start on the backlog/deficit of unit-tests highlighted in #966.

Making a start on the backlog/deficit of unit-tests highlighted in #966.
@ns-rse ns-rse added the tests Issues pertaining to testing label Oct 22, 2024
pytest.param((10, 10), [7, 7, 9, 9], 4, (3, 3), id="Simple square padded by 4 beyond bottom and right edges"),
],
)
def test_grain_anchor(array_shape: tuple, bounding_box: list, pad_width: int, target: npt.NDArray) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is grain_anchor used anywhere? I pulled the branch and can't find any references to where it's used
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure, I recall I introduced it because we needed a way of putting tracing, done on cropped grains, back into the original image and the solution I came up with was to have an anchor point so we could put it back in the right location.

If its been removed in the refactor in favour of an alternative method of putting everything back that is perfectly fine but there was too much in the code/pull request for me to spot that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well it's tested and imo potentially useful so let's keep it. Was just wondering 😄

@@ -21,7 +21,8 @@

LOGGER = logging.getLogger(LOGGER_NAME)

# pylint: disable=too-many-positional-arguments
# too-many-positional-arguments
# pylint: disable=R0917
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah nice! I've been looking for a way to suppress this warning as the # pylint: disable=too-many-positional-arguments doesn't seem to work!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its quite annoying as its a valid name but doesn't seem to be recognised. I should raise an issue but still have to report to the Topoloy developers the issues with Python version releases and other problems uncovered the other day and don't have time for everything.

@ns-rse ns-rse added this pull request to the merge queue Oct 23, 2024
Merged via the queue into main with commit 355c089 Oct 23, 2024
11 checks passed
@ns-rse ns-rse deleted the ns-rse/966-disordered-tracing branch October 23, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Issues pertaining to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants