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

Fix numpy doctest #176

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix numpy doctest #176

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f44dad8
np.float_ → np.float64
floesche Dec 11, 2024
bd0e2dc
workaround for doctest comparison of np dtypes
floesche Dec 11, 2024
d650c2b
Merge branch 'navis-org:master' into fix_numpy_doctest
floesche Dec 16, 2024
3f2d47e
move number representation to test itself
floesche Dec 16, 2024
f5b45ab
set test env to switch numpy representation
floesche Dec 16, 2024
628c0c1
commiting the changes I described earlier in #176
floesche Dec 16, 2024
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
6 changes: 6 additions & 0 deletions navis/morpho/mmetrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@
# Set up logging
logger = config.get_logger(__name__)

# Set up numpy number representation, see NEP51
# Once numpy<=2 is dropped from requirements, the doctest comparissons
# should become `np.float64(1.074)` instead of `1.074`
if int(np.version.version.split('.')[0])>=2:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would put this in config.py - feels more appropriate when adjusting global settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the suggested changes.

In this scenario, the whole code basis gets switched to a different representation of np values. This means, in theory there could a piece of code that behaves differently in the NAVIS_TEST_ENV than it does whenever you run the code without that variable set to true. I can't think of a case where this would matter. Since you were worried about "global" changes to the representation / print option, I also made an alternative PR #177, where the change of number representation should only change within the pydoctest.

Either #176 or #177 should be accepted, not both.

np.set_printoptions(legacy="1.25")

__all__ = sorted(
[
"strahler_index",
Expand Down
4 changes: 2 additions & 2 deletions navis/plotting/plot_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def make_tube(segments, radii=1.0, tube_points=8, use_normals=True):
faces : np.ndarray

"""
vertices = np.empty((0, 3), dtype=np.float_)
vertices = np.empty((0, 3), dtype=np.float64)
indices = np.empty((0, 3), dtype=np.uint32)

if not isinstance(radii, Iterable):
Expand Down Expand Up @@ -219,7 +219,7 @@ def make_tube(segments, radii=1.0, tube_points=8, use_normals=True):
# Vertices for each point on the circle
verts = np.repeat(points, tube_points, axis=0)

v = np.arange(tube_points, dtype=np.float_) / tube_points * 2 * np.pi
v = np.arange(tube_points, dtype=np.float64) / tube_points * 2 * np.pi

all_cx = (
radius
Expand Down
Loading