Skip to content

Refactor/310 plus typehints#1522

Closed
jlnav wants to merge 49 commits intodevelopfrom
refactor/310_plus_typehints
Closed

Refactor/310 plus typehints#1522
jlnav wants to merge 49 commits intodevelopfrom
refactor/310_plus_typehints

Conversation

@jlnav
Copy link
Copy Markdown
Member

@jlnav jlnav commented Feb 21, 2025

With Python 3.9 dropped, many type hints can be modernized, e.g:

Optional[Union[int, float]] -> int | float | None

Changes were guided by flake8-modern-annotations and flake8-type-checking,
which were added to the dev environment.

Addresses #1378

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 83.74384% with 33 lines in your changes missing coverage. Please review.

Project coverage is 76.93%. Comparing base (2d31119) to head (d802ad3).
Report is 140 commits behind head on develop.

Files with missing lines Patch % Lines
libensemble/gen_funcs/persistent_ax_multitask.py 0.00% 14 Missing ⚠️
libensemble/resources/platforms.py 75.00% 6 Missing and 1 partial ⚠️
libensemble/executors/balsam_executor.py 0.00% 3 Missing ⚠️
libensemble/resources/mpi_resources.py 66.66% 2 Missing and 1 partial ⚠️
libensemble/resources/rset_resources.py 60.00% 1 Missing and 1 partial ⚠️
libensemble/resources/worker_resources.py 71.42% 1 Missing and 1 partial ⚠️
libensemble/worker.py 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1522      +/-   ##
===========================================
- Coverage    78.27%   76.93%   -1.35%     
===========================================
  Files           76       77       +1     
  Lines         7674     7816     +142     
  Branches      1146     1160      +14     
===========================================
+ Hits          6007     6013       +6     
- Misses        1469     1602     +133     
- Partials       198      201       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jlnav jlnav requested review from jmlarson1 and shuds13 February 21, 2025 17:28
Comment thread pyproject.toml
pre-commit = ">=4.0.1,<5"
nlopt = ">=2.8.0,<3"
scipy = ">=1.9.1,<2"
ipdb = ">=0.13.13,<0.14"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is ipdb now a dependency?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope, I just added it to the dev virtual environment.

self.gen_ngpus = libE_info.get("num_gpus")

def set_resources(self, resources: Resources) -> None:
def set_resources(self, resources) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

whats the idea here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't necessarily need to import the class just for typehint purposes

data: Data,
search_space: Optional[SearchSpace] = None,
trial_index: Optional[int] = None,
search_space: Optional[SearchSpace] = None, # noqa: MDA501
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

whats this about


@abstractmethod
def live_update(self, hist: npt.NDArray) -> None:
def live_update(self, hist: object) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems too general

return Executor

def _result(self, calc_in: npt.NDArray, persis_info: dict, libE_info: dict) -> (npt.NDArray, dict, Optional[int]):
def _result(self, calc_in: npt.NDArray, persis_info: dict, libE_info: dict) -> (npt.NDArray, dict, int | None):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

npt.NDArray still here but not above?

@shuds13
Copy link
Copy Markdown
Member

shuds13 commented Mar 18, 2025

There are still some places public facing functions without type hints (like tools.py). is there the some method to what has hints or not.

@jlnav
Copy link
Copy Markdown
Member Author

jlnav commented Mar 20, 2025

Merging develop into this. Warning ahead of time that the upcoming commit will contain fixes to address:

libensemble/gen_funcs/persistent_ax_multitask.py:59:1: F811 redefinition of unused 'assert_is_instance' from line 38
libensemble/gen_funcs/persistent_ax_multitask.py:63:5: F811 redefinition of unused 'MBM_X_trans' from line 47
libensemble/gen_funcs/persistent_ax_multitask.py:64:5: F811 redefinition of unused 'ConvertMetricNames' from line 49
libensemble/gen_funcs/persistent_ax_multitask.py:65:5: F811 redefinition of unused 'Derelativize' from line 50
libensemble/gen_funcs/persistent_ax_multitask.py:66:5: F811 redefinition of unused 'StratifiedStandardizeY' from line 51
libensemble/gen_funcs/persistent_ax_multitask.py:67:5: F811 redefinition of unused 'TaskChoiceToIntTaskChoice' from line 52
libensemble/gen_funcs/persistent_ax_multitask.py:68:5: F811 redefinition of unused 'TrialAsTask' from line 53
libensemble/gen_funcs/persistent_ax_multitask.py:99:1: F811 redefinition of unused 'MT_MTGP_trans' from line 80

jmlarson1 and others added 28 commits April 8, 2025 12:09
…e attribute from the object. try the dynamic versioning again...
…ibensemble.gen_funcs.persistent_tasmanian import *
Replace flashing live animation with clean post-run version
Testing/some coverage + another attempt at dynamic versioning
@jlnav jlnav requested a review from shuds13 April 11, 2025 19:09
@jlnav jlnav closed this Apr 22, 2025
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