Add docstrings and expand tests for IO sub-module

[K-Meech]

Description

What is this PR

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?
There are very few docstrings for functions in the IO sub-module. Also some parts aren't covered by tests.

What does this PR do?
This PR adds docstrings for all functions in the IO sub-module. It also expands/refactors the tests in test_cell_io

References

For #19

How has this PR been tested?

All tests pass locally.

Is this a breaking change?

No

Does this PR require an update to the documentation?

No

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.48%. Comparing base (cacefd6) to head (333e05a).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
+ Coverage   86.49%   90.48%   +3.99%     
==========================================
  Files          35       35              
  Lines        1355     1356       +1     
==========================================
+ Hits         1172     1227      +55     
+ Misses        183      129      -54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[willGraham01]

Great stuff - one pathlib.Path convention throughout that I've flagged where I saw it.

Only thing that I'd like some clarity on is the values we're passing to the tests. Seems like a lot of repetition in the hard-coded values - not sure if they can be reduced any further though?

[willGraham01]

Suggested change

	import pathlib
	from pathlib import Path

As far as I can tell we only use Path, and the rest of the BG codebase errs towards from pathlib import Path.

[willGraham01]

Suggested change

	def cells_to_csv(cells: List[Cell], csv_file_path: Union[str, pathlib.Path]):
	"""Save cells to csv file"""
	def cells_to_csv(cells: List[Cell], csv_file_path: Union[str, Path]):
	"""Save cells to csv file"""

To be consistent with the import change above.

Also there's a trailing newlines between the docstring and first command in the function which isn't present in the others.

[willGraham01]

Suggested change

	xml_file_path : str or pathlib.Path
	xml_file_path : str or Path

[willGraham01]

Noticed another pathlib.Path here, but in another file. Could you double-check if we're using anything else from pathlib in this file?

[willGraham01]

(Will's pathlib.Path rant continues)

[K-Meech]

@willGraham01 I've looked through brainglobe_utils and turns out there are many files that use pathlib.Path rather than Path. Are you happy for me to merge this PR as-is, then make a separate PR to fix all the Path issues at once? Otherwise, it will introduce changes across files outside the IO sub-module.

[willGraham01]

Are you happy for me to merge this PR as-is, then make a separate PR to fix all the Path issues at once? Otherwise, it will introduce changes across files outside the IO sub-module.

Yep happy for this to go in as it is. I'll open a PR of my own to change all the pathlib.Path syntax across the package since that's become a pet peeve of mine 😅