DiamondLightSource / hdfmap

Map objects within an HDF file and create a dataset namespace
Apache License 2.0
2 stars 0 forks source link

Quick code review #1

Closed PeterC-DLS closed 3 months ago

PeterC-DLS commented 4 months ago

Some issues from a quick review of the code without examining its semantics or coding style

Have you used ruff to check and format it? And mypy or pylance to analyse it?

DanPorter commented 4 months ago

Thanks very much @PeterC-DLS for the code review!

I've created a branch and made the changes you've suggested. The pull request #2 is now available for you to check.

I've primarily used the inbuilt linter/ analysis in PyCharm for the project, but I've checked it against ruff and mypy as well.

Interestingly PyCharm's inbuilt type checking and the mypy plugin show now errors, but if you run mypy directly it still gives a load, although the are pretty insignificant so I've stuck to the built in ones.

DanPorter commented 3 months ago

Merged pull request #2, all issues addressed. Some additional comments addressed in #4