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

Offer pytensor.tensor user-facing functionality at the pytensor root level #635

Open
ricardoV94 opened this issue Feb 6, 2024 · 10 comments

Comments

@ricardoV94
Copy link
Member

ricardoV94 commented Feb 6, 2024

Description

Make import pytensor as pt basically work like today's import pytensor.tensor as pt.

For users that's all they need, besides functionality like scan, ifelse, function, shared that should also be available at the root level (see #332)

Internally things can still be implemented inside the tensor submodule.

Does anyone have strong objections or see why this may not be possible?

@ricardoV94 ricardoV94 changed the title Offer pytensor.tensor functionality at the pytensor root level Offer pytensor.tensor user-facing functionality at the pytensor root level Feb 6, 2024
@maresb
Copy link
Contributor

maresb commented Feb 6, 2024

I'm weakly 👎.

I don't see any technical obstacles. But is there any benefit beyond saving users a few keystrokes?

This would introduce yet another layer of indirection. It's already pretty tricky to find the definitions of stuff. Like to find the definition of pt.dscalar I have to To look in __init__.py, and then I search for dscalar and there's no result, because it's actually pulled in implicitly pytensor.tensor.type import *. There are lots of other import * statements, so the only way I could find out which one was with a raw text search.

With various linters and type checkers I've gotten into the habit of always importing things directly from their definition, and so my personal feeling is that these types of shortcuts cost me way more time than they save.

TLDR: keystrokes are cheap, directness and clarity are priceless

Counterproposal: eliminate *-imports from the codebase.

@ricardoV94
Copy link
Member Author

ricardoV94 commented Feb 6, 2024

Writing pytensor.tensor is a PITA (what else would I want to use from a library with tensor in the name?), only surpassed by matplotlib.pyplot. And I have to write both everytime.

Importing from source is fine for dev work but not user work, such as Jupyter notebook workflows.

Why should the main functionality of a package require a double level import? I can't believe this would be suggested if we were writing this library from scratch.

@ricardoV94
Copy link
Member Author

The counter proposal doesn't make sense to me.

Are you suggesting one to do from pytensor.tensor.math/basic/elemwise import add, dot, cos, zeros...?

@maresb
Copy link
Contributor

maresb commented Feb 6, 2024

Importing from source is fine for dev work but not user work, such as Jupyter notebook workflows.

Ya, that's actually a really good argument. I've been doing too much library dev work. Now I'm weakly 👍.

It still would be nice to get rid of the * imports.

@twiecki
Copy link
Member

twiecki commented Feb 6, 2024

I'm strongly 👍.

@jessegrabowski
Copy link
Member

I like both the proposal and the counter-proposal.

While I don't love it, I'll also throw into the mix the import statsmodels.api as sm pattern. They provide the api module as a way to organize and present all the user-facing stuff in a clean way, while still letting them organize the rest of the package in a dev friendly way.

@ricardoV94
Copy link
Member Author

@jessegrabowski can you expand on what the api module does? I am not familiar with it

@jessegrabowski
Copy link
Member

It's just a script that imports from around their modules in a user-facing way.

The consequence is that you have to do import statsmodels.api as sm, but the benefit is they can organize the package in a way that makes sense for the dev team, then present it to users in a way that makes sense to them.

@twiecki
Copy link
Member

twiecki commented Feb 8, 2024

FWIW I always found statsmodels' import structure pretty odd and besides matplotlib I don't know another library that does it that way.

@maresb
Copy link
Contributor

maresb commented Feb 8, 2024

Just to brainstorm likely-bad ideas, I wonder if it would make sense to flip the statsmodels pattern upside down. Make the top-level pytensor namespace the user API. Then in order to ensure separation between the user API and the development library, we could move the actual code into pytensor.lib. That way we ensure no name conflicts, and the user can tab-complete all user API names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants