Skip to content

Conversation

@alexfikl
Copy link
Collaborator

@alexfikl alexfikl commented Nov 16, 2024

Fixes #246.

@alexfikl alexfikl marked this pull request as ready for review November 17, 2024 19:13
@inducer
Copy link
Owner

inducer commented Nov 17, 2024

Thanks! One thing that went through my head is whether we should outright deprecate direct constructor calls that bypass the factory functions.

@inducer
Copy link
Owner

inducer commented Nov 17, 2024

Implied but not said: how do you feel about that idea?

@alexfikl
Copy link
Collaborator Author

Thanks! One thing that went through my head is whether we should outright deprecate direct constructor calls that bypass the factory functions.

Calling the constructor directly should still be ok for "internal" code though, right? e.g. all the mappers call the constructor directly.

@alexfikl alexfikl force-pushed the fix-componentwise branch 2 times, most recently from c7a184e to 9f471cf Compare November 18, 2024 08:03
@inducer
Copy link
Owner

inducer commented Nov 18, 2024

Calling the constructor directly should still be ok for "internal" code though, right? e.g. all the mappers call the constructor directly.

Yes, definitely. One question is whether we'll make them pass a flag to say, "I'm internal!"

@alexfikl
Copy link
Collaborator Author

Yes, definitely. One question is whether we'll make them pass a flag to say, "I'm internal!"

I don't know, I agree that we should discourage that somehow, but maybe a little blurb in the docs will do the trick? I'm mostly hesitant because: adding a little flag is annoying + none of the other expressions have a deprecated constructor.

@inducer inducer merged commit 223ab83 into inducer:main Nov 25, 2024
9 checks passed
@inducer
Copy link
Owner

inducer commented Nov 25, 2024

This is awesome. Thanks so much for doing this!

@alexfikl alexfikl deleted the fix-componentwise branch November 25, 2024 06:18
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.

Discussion: Deprecate overloading __new__ for operator application?

2 participants