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.

Sign up for GitHub

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

Jump to bottom

ENH: Function to detect sign of a number #14

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ENH: Function to detect sign of a number #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions project_name/feature_sign.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def sign_func(value: float) -> float:
return -1 if value < 0 else 1
23 changes: 23 additions & 0 deletions project_name/tests/test_sign.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import pytest
Copy link
Contributor

Choose a reason for hiding this comment

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

There's one more thing failing in the "pre-commit" check.

Did you use pre-commit when committing your code? It automatically sorts imports and tells you about style issues before it gets to GitHub.

The output here:

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /home/runner/work/pcds-ci-test-repo-python/pcds-ci-test-repo-python/project_name/tests/test_sign.py

pre-commit hook(s) made changes.
If you are seeing this message in CI, reproduce locally with: `pre-commit run --all-files`.
To run `pre-commit` as part of git workflow, use `pre-commit install`.
All changes made by hooks:
diff --git a/project_name/tests/test_sign.py b/project_name/tests/test_sign.py
index 6fc37e4..14c7ffc 100644
--- a/project_name/tests/test_sign.py
+++ b/project_name/tests/test_sign.py
@@ -1,5 +1,5 @@
-import pytest
 import numpy as np
+import pytest

Tells you that isort would have put numpy before pytest, because they are sorted alphabetically by convention.

To fix it, you can do as it says above:

$ pre-commit install
$ pre-commit run --all-files

Or manually fix based on what it says, of course. I'm lazy and prefer to have pre-commit take care of it for me, though!

import numpy as np
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you see in the test suite here that things are failing, saying numpy cannot be found?

You might wonder why it works while using pcds_conda but not here.
That's because each of our packages have their own set of requirements. We first test them in isolation - not as part of the conda environment we use on a daily basis.

You can fix this by adding numpy as a dependency. We specify dependencies in a couple spots for the package manager pip (ignoring the conda builds for now):

  1. For "normal" requirements, required to run a package as a user, these would go in requirements.txt.
  2. For requirements that are used only in the test suite, those go in dev-requirements.txt ("dev" for "development", of course)

Since numpy is only used in your test suite in this PR, it should go in dev-requirements.txt. If you open that file, you'll see that pytest is listed there currently, because it's what we use to run the test suite.


from ..feature_sign import sign_func


def test_simple_sign_func():
assert sign_func(-100000.346) == -1


@pytest.mark.parametrize(
"value, expected",
[
(0, 1),
(10000.0, 1),
(-345236.78, -1)
]
)

def test_sign_func(value: float, expected: float):
sign_val = sign_func(value)
expected = np.sign(value)
assert expected==sign_val