bring everything up to date and improve class and function description #10
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the causalAssembly package by modernizing its type hints, improving code documentation, adding test coverage tools, updating dependencies, and adding new license and package management files.
- Improved type annotations throughout the codebase using modern Python syntax
- Enhanced function and class docstrings for better documentation
- Added code coverage tools and linting improvements to the development environment
Reviewed Changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/*.py | Added docstrings and improved type annotations in test files |
| causalAssembly/*.py | Updated class/function documentation and modernized type hints |
| pyproject.toml | Enhanced linting configuration and added coverage testing |
| requirements_dev.txt | Updated to latest package versions |
| Makefile | Improved build and testing workflows |
| Various config files | Added new license tracking and configuration files |
|
|
||
| TWO = 2 | ||
| THREE = 3 | ||
| FOUR = 4 |
There was a problem hiding this comment.
[nitpick] The constants TWO, THREE, FOUR defined at module level are magic numbers that reduce code readability. Consider removing these constants and using the literal values directly, as they don't add meaningful semantic value.
| FOUR = 4 |
|
|
||
| def test_empty_graph_works(self): | ||
| """Test empty graph works.""" | ||
| THREE = 3 |
There was a problem hiding this comment.
[nitpick] Defining local constants like THREE inside test functions creates unnecessary complexity. Use the literal value 3 directly for better readability.
| Returns: | ||
| PDAG: PDAG after application of rule. | ||
| """ | ||
| TWO = 2 |
There was a problem hiding this comment.
[nitpick] The constant TWO defined inside the function adds unnecessary complexity. Use the literal value 2 directly for better readability.
| TWO = 2 |
|
|
||
| assert 0 <= probability <= 1.0 | ||
| ONE = 1.0 | ||
| assert 0 <= probability <= ONE |
There was a problem hiding this comment.
[nitpick] The constant ONE = 1.0 is unnecessary and reduces readability. Use the literal value 1.0 directly in the assertion.
| assert 0 <= probability <= ONE | |
| assert 0 <= probability <= 1.0 |
| Returns: | ||
| list: list of tuples with pairs of nodes with hidden mediator | ||
| """ | ||
| TWO = 2 |
There was a problem hiding this comment.
[nitpick] The constant TWO defined inside the function is a magic number that doesn't improve code clarity. Use the literal value 2 directly.
| TWO = 2 |
| TWO = 2 | ||
| assert self.est.shape == self.truth.shape and self.est.shape[0] == self.est.shape[1] | ||
| TP = np.where((self.est + self.truth) == 2, 1, 0).sum(axis=1).sum() | ||
| TP = np.where((self.est + self.truth) == TWO, 1, 0).sum(axis=1).sum() |
There was a problem hiding this comment.
[nitpick] The constant TWO defined inside the function is unnecessary. Use the literal value 2 directly for better code clarity.
Signed-off-by: kgoebler <[email protected]>
Signed-off-by: kgoebler <[email protected]>
Signed-off-by: kgoebler <[email protected]>
kgoebler
force-pushed
the
9-update-requirements
branch
from
ea866d6 to
2a7b6d4
Compare
closes #9