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

Add numeric_bounds function, and do some related refactoring #62

Merged
merged 9 commits into from
Dec 6, 2023

Conversation

ctrueden
Copy link
Member

This patchset:

  1. migrates secondary Java-type-related functionality into a new internal _types.py, and renames the _java.py to _jvm.py;
  2. adds new functions for testing if a Java type is a (boxed) primitive;
  3. adds a new numeric_bounds function for getting the (min, max) of each particular type.

It is intended to be generally useful, but the motivating use case was to support solving imagej/napari-imagej#276.

@ctrueden ctrueden requested a review from gselzer July 26, 2023 18:31
Copy link
Member

@gselzer gselzer left a comment

Choose a reason for hiding this comment

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

Looks great to me!

src/scyjava/__init__.py Outdated Show resolved Hide resolved
@ctrueden
Copy link
Member Author

@gselzer Thanks for the review. Have you actually tested it with napari-imagej? Will it meet your needs to facilitate solving imagej/napari-imagej#276? I'd like to wait to merge till you've actually successfully used the API to improve the magic-gui experience.

@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2023

Codecov Report

Attention: 84 lines in your changes are missing coverage. Please review.

Comparison is base (5be0963) 52.38% compared to head (112c4bd) 53.15%.

Files Patch % Lines
src/scyjava/_types.py 51.23% 59 Missing ⚠️
src/scyjava/_jvm.py 0.00% 14 Missing ⚠️
src/scyjava/__init__.py 0.00% 6 Missing ⚠️
src/scyjava/_convert.py 0.00% 3 Missing ⚠️
src/scyjava/_versions.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   52.38%   53.15%   +0.77%     
==========================================
  Files          10       12       +2     
  Lines        1218     1285      +67     
==========================================
+ Hits          638      683      +45     
- Misses        580      602      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gselzer
Copy link
Member

gselzer commented Jul 26, 2023

Haven't tried it out yet, I'll try to take a look tomorrow!

Absolute imports are the Pythonic recommendation in general.
However, for the toplevel __init__.py, relative imports are
acceptable, and perhaps even preferred for succinectness.
Instead, we use Float/Double.MAX_VALUE.
Because the nicest modular Pythonic design is to import all your public
API from supporting files into __init__.py, so they are available from
the toplevel, and we don't want flake8 yelling at us about it.

Thanks to @gselzer and https://stackoverflow.com/a/58029222/1207769.
@ctrueden ctrueden merged commit 4ef3781 into main Dec 6, 2023
12 of 13 checks passed
@ctrueden ctrueden deleted the numeric-bounds branch December 6, 2023 18:28
@ctrueden
Copy link
Member Author

ctrueden commented Dec 6, 2023

@gselzer I know you're busy with other things these days, so I took the liberty of rebasing and merging this PR. It will be part of scyjava 1.10.0.

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.

3 participants