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

[develop] ♻️ Refactored code for better testability and added tests 🧪 #7

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

jsnel
Copy link
Member

@jsnel jsnel commented Jul 27, 2024

Targetting develop branch from which PR #5 was opened targeting main.

  • ♻️ Refactored several functions to improve readability and maintainability
  • ♻️Replaced os module with pathlib for file operations 👌
  • 📚Improved docstrings for clarity and consistency and satify linters 👌
  • ♻️ Renamed 'widget' to '_widget' in widget.py to facilitate unit testing 🧪
  • 🧪Added a new test case for the _simulate function in widget.py -

- Refactored several functions to improve readability and maintainability
- Replaced os module with pathlib for file operations
- Improved docstrings for clarity and consistency and satify linters
- Renamed 'widget' to '_widget' in widget.py to facilitate unit testing
- Added a new test case for the _simulate function in widget.py
-
@jsnel jsnel requested a review from anmolbhatia05 July 27, 2024 11:39
@jsnel
Copy link
Member Author

jsnel commented Jul 27, 2024

To evaluate this PR, place the attached (zipped) notebook, in the examples folder.
getting_started_with_pyparamgui.zip

This is indeed why I have added these lines to .gitignore:

examples/
!examples/*.ipynb

I found it handy to track changes locally, but they can also be removed again.

We can decide whether or not to add an example notebook later, or just add something to the docs.

@jsnel
Copy link
Member Author

jsnel commented Jul 27, 2024

Note the rename of the widget instance to _widget. If you rename this back you will run into the following error during testing:

FAILED tests/test_simulate.py::test_simulate - ZeroDivisionError: float division by zero

The reason for that is that during the (simulate) test, it will use an uninitialized instance of widget, which has all zero or empty values. Renaming the variable avoids this, but it would be better if no global (at import time) instantiation of the widget instance is needed.

Also ideally the widget observer wouldn't need to be initialized separately (as in the previously attached example notebook) but is part of the init.

@anmolbhatia05 anmolbhatia05 merged commit 769f177 into glotaran:develop Jul 27, 2024
18 checks passed
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.

2 participants