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

Use inspect in order to get type hint metadata #570

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

samwaseda
Copy link
Member

I was trying to get full type hints by using inspect instead of get_type_hints.

Well, this being said I'm painfully aware of the fact that the tests fail, but I don't really know why XD The problem comes from the fact that somehow type_dict["return"] turns the return type into a string, so for example instead of tuple[float, float, float] you get "tuple[float, float, float]", even though if I copy and paste the code snippet I can get the correct type. Is there any transformation that takes place before preview_io?

Copy link

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_workflow/preview_io

@samwaseda samwaseda marked this pull request as draft January 23, 2025 15:34
@liamhuber
Copy link
Member

I don't think I'll have the time to support and dig deep on this until the end of next week, but to try to help a little at first glance: the error looks as though the code want's a tuple of hints (even if they're None) but the new routine provides an empty tuple. Since you're replacing one standard library call for a different one (plus some manipulation), I guess that for now it's just a matter of getting the new call + manipulation to conforming to the return type of the old one -- i.e. I don't expect complications from pyiron_workflow, at this stage it should all be totally local interactions with the standard library.

Is there any transformation that takes place before preview_io?

No, what you're doing here is the transformation. Subclasses of classes like Function and Macro get the io-defining function assigned to them, and here we directly parse it. From what I see here/recall, there is no pyiron_workflow magic here; per above you're just passing some function to the standard library inspection tools.

@samwaseda
Copy link
Member Author

I don't think I'll have the time to support and dig deep on this until the end of next week

That's fine. I was anyway going to work on it myself. I just thought you might know where the problem comes from.

Maybe I make it a bit more specific what I wanted to say in my post above: The following snippet delivers the same results for a given function f as get_type_hints(f):

sig = inspect.signature(f)
type_dict = {
    key: value.annotation for key, value in sig.parameters.items()
}
type_dict = type_dict | {"return": sig.return_annotation}
type_dict = {
    k: v for k, v in type_dict.items() if v != inspect.Parameter.empty
}

The weird thing is, inside pyiron_workflow somehow "return": sig.return_annotation is turned into a string. So in my notebook, I get the error message about the following function:

@as_function_node("started_at", "ended_at", "was_empty")
def ChangeDirectory(
    path: str | Path,
    delete_start: bool = False,
) -> tuple[str, str, bool]:

and tuple[str, str, bool] is turned into "tuple[str, str, bool]", then by applying get_args it's turned into an empty tuple. So the symptoms can be explained. I just don't understand the cause.

Anyway I was thinking about working on it myself, so if nothing comes to your mind, just leave it like this.

@samwaseda
Copy link
Member Author

Thanks to this post I found out the reason - apparently from __future__ import annotations messes up the things. There was an argument eval_str that I had to set to True. Now it should be fine.

Copy link

codacy-production bot commented Jan 24, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.01% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (e316d6e) 3425 3130 91.39%
Head commit (9a682cf) 3430 (+5) 3135 (+5) 91.40% (+0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#570) 8 8 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@coveralls
Copy link

coveralls commented Jan 24, 2025

Pull Request Test Coverage Report for Build 12947532852

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 18 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.01%) to 91.399%

Files with Coverage Reduction New Missed Lines %
nodes/for_loop.py 2 98.19%
nodes/macro.py 7 92.25%
mixin/preview.py 9 93.29%
Totals Coverage Status
Change from base Build 12924745094: 0.01%
Covered Lines: 3135
Relevant Lines: 3430

💛 - Coveralls

@samwaseda samwaseda added the format_black trigger the Black formatting bot label Jan 24, 2025
@samwaseda samwaseda marked this pull request as ready for review January 24, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black trigger the Black formatting bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants