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

Standardise package structure and add tests #57

Merged
merged 24 commits into from
Dec 20, 2023
Merged

Standardise package structure and add tests #57

merged 24 commits into from
Dec 20, 2023

Conversation

adamltyson
Copy link
Member

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
This package followed the napari cookiecutter, not the neuroinformatics one (as with the rest of BrainGlobe). While standardisation with the rest of the napari plugin ecosystem was good, I thought it was much more likely that NIU members will maintain this plugin rather than members of the wider napari ecosystem.

There was also a lack of tests (#7)
What does this PR do?
Restructures the package and updates the tooling to match the rest of the repos and adds some more tests.

References

Closes #7

How has this PR been tested?

Tests added

Is this a breaking change?

Nope

Does this PR require an update to the documentation?

Nope

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit


def test_xml_write_no_metadata(tmp_path):
xml_path = str(tmp_path / "points.xml")
rng = np.random.default_rng()

Check notice

Code scanning / SonarCloud

Results that depend on random number generation should be reproducible Low test

Provide a seed for this random generator. See more on SonarCloud
Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

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

Not sure about a few very minor things, but should be good once you've considered.

.pre-commit-config.yaml Outdated Show resolved Hide resolved
tests/tests/test_integration/test_cellfinder_reader.py Outdated Show resolved Hide resolved
brainglobe_napari_io/napari.yaml Show resolved Hide resolved
@adamltyson adamltyson merged commit 3ca66b2 into main Dec 20, 2023
23 checks passed
@adamltyson adamltyson deleted the update branch December 20, 2023 14:38
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.

Add some integration tests
2 participants