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

create_temporary_modules should error out with helper (_) functions #510

Closed
elijahbenizzy opened this issue Oct 31, 2023 · 3 comments
Closed

create_temporary_modules should error out with helper (_) functions #510

elijahbenizzy opened this issue Oct 31, 2023 · 3 comments
Labels
triage label for issues that need to be triaged.

Comments

@elijahbenizzy
Copy link
Collaborator

If we create_temporary_modules with an underscore function it should not work. It will not get turned into a function, and the function can refer to a helper function regardless.

This can help the ad-hoc case.
@elijahbenizzy elijahbenizzy added the triage label for issues that need to be triaged. label Oct 31, 2023
@skrawcz
Copy link
Collaborator

skrawcz commented Oct 31, 2023

I don't understand? You pass in the functions with create_temporary_modules? Why not just prevent people passing in _ ones?

@elijahbenizzy
Copy link
Collaborator Author

I don't understand? You pass in the functions with create_temporary_modules? Why not just prevent people passing in _ ones?

Yep, that's what this issue suggests. It curerntly does work, and gets weird results, but it shouldn't.

elijahbenizzy added a commit that referenced this issue Nov 27, 2023
@elijahbenizzy
Errors out when calling create_temporary_module with functions that
2cb73a2 
start with "_"

See #510

This is *not* backwards compatible, but this is a specific "ad-hoc" util
and has an easy fix, so I'm OK not bumping versions accordingly. I would
consider this previously undefined behavior, and this will make
debugging easier.
@elijahbenizzy elijahbenizzy mentioned this issue Nov 27, 2023
Merged
7 tasks
elijahbenizzy added a commit that referenced this issue Nov 27, 2023
@elijahbenizzy
Errors out when calling create_temporary_module with functions that
aaf966f 
start with "_"

See #510

This is *not* backwards compatible, but this is a specific "ad-hoc" util
and has an easy fix, so I'm OK not bumping versions accordingly. I would
consider this previously undefined behavior, and this will make
debugging easier.
elijahbenizzy added a commit that referenced this issue Nov 27, 2023
@elijahbenizzy
Errors out when calling create_temporary_module with functions that
e7d8ebc 
start with "_"

See #510

This is *not* backwards compatible, but this is a specific "ad-hoc" util
and has an easy fix, so I'm OK not bumping versions accordingly. I would
consider this previously undefined behavior, and this will make
debugging easier.
elijahbenizzy added a commit that referenced this issue Nov 27, 2023
@elijahbenizzy
Errors out when calling create_temporary_module with functions that
7886a67 
start with "_"

See #510

This is *not* backwards compatible, but this is a specific "ad-hoc" util
and has an easy fix, so I'm OK not bumping versions accordingly. I would
consider this previously undefined behavior, and this will make
debugging easier.
@elijahbenizzy
Copy link
Collaborator Author

This is complete! #558

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
triage label for issues that need to be triaged.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@skrawcz @elijahbenizzy