How could classes be made more accessible?

[michaelosthege]

Here's a list of submodules/types/functions that currently need their own import lines, but are so frequently used that I think we should aggregate them at higher package/submodule levels:

from aesara.graph.basic import Constant, Variable
from aesara.tensor.var import TensorVariable
from aesara.compile.sharedvalue import SharedVariable
from aesara.tensor.sharedvar import ScalarSharedVariable
from aesara.graph.op import Op

import aesara.tensor as at
import aesara.tensor.random.basic as atr

While the above is not something I'd like to show others when demoing Aesara, this looks much less frightening:

from aesara import Constant, Variable, TensorVariable, SharedVariable, ScalarSharedVariable, Op
import aesara.tensor as at
import aesara.tensor.random as atr

or simply this and rolling with aesara.TensorVariable and so on:

import aesara
import aesara.tensor as at
import aesara.tensor.random as atr

Grouping the most important types at the package level also doesn't require in-depth knowledge of the codebase just to import things for an isinstance check.

Any concerns, or more things to add to the list?

@ricardoV94 @brandonwillard can someone add labels, please? I seem to have absolutely no permissions on this project any more :(

[brandonwillard]

Some of the basic classes could be exposed at higher package levels, but not all (e.g. it makes little-to-no sense to expose the *SharedVariable classes) and definitely not at the highest package level (i.e. aesara), as your examples do.

One of the most fundamental deciding factors is whether or not a class is intended to be used directly, and none of the classes in your example fit that simple criteria.

Other reasonable considerations involve the types of operations user-level code would be expected to do with Aesara objects, and these needs to be considered with the following question in mind: "Are we still talking about users, or are we now talking about developers?".

While ease-of-isinstance-use may seem like one, it can easily devolve into a justification for exposing everything at the top level, so that's not good enough. At most, Variable could be at the top level and TensorVariable at aesara.tensor. That's enough to address most reasonable isinstance cases.

While the above is not something I'd like to show others when demoing Aesara, this looks much less frightening:
...
Grouping the most important types at the package level also doesn't require in-depth knowledge of the codebase just to import things for an isinstance check.

These are specious points/arguments. The first one is simply FUD, and it's not welcome here. I'm assuming the second is hyperbole, because, when one considers the circumstances under which a user would even determine that they need TensorVariable in the first place, the claim makes no sense. (For example, if they used type(...) at any point, then they know exactly where the class is.)

[ricardoV94]

By the way, I find the aesara.tensor.var module a bit difficult to find because it "conflicts" with the op aesara.tensor.var

[brandonwillard]

Yeah, this is one reason why exposing some classes from aesara.tensor could be an improvement, or—better yet—the module could be renamed or merged with aesara.tensor.type.

The var.py module naming is only used in one other place, and the base Variable is in a basic.py module, so the var convention isn't very solid and could use some reconsideration.

[ricardoV94]

Yeah, putting it in the type module might make sense as well

[michaelosthege]

I don't like to show the original examples when demoing Aesara, because they demonstrate that currently one needs to know a lot about the internal structure of the codebase to work with this library.
For example, import aesara.tensor.random.basic is a level deeper into the submodule tree than one would expect from the NumPy equivalent. Likewise from aesara.tensor.var import TensorVariable is an import that is so unintuitive I have to look it up myself all the time.

The first one is simply FUD, and it's not welcome here.

Not cool to accuse me of propaganda techniques that I even have to look up. I am contributing these suggestions for improvement with positive intent, but responses like that and categorical dismissal of my contributions (e.g. labeling "wontfix" before even waiting for a third opinion) really make me feel unwelcome here.

[brandonwillard]

I don't like to show the original examples when demoing Aesara, because they demonstrate that currently one needs to know a lot about the internal structure of the codebase to work with this library.

They demonstrate nothing of the sort. They only show the minor inconvenience of not being able to import everything from the top-level.

For example, import aesara.tensor.random.basic is a level deeper into the submodule tree than one would expect from the NumPy equivalent. Likewise from aesara.tensor.var import TensorVariable is an import that is so unintuitive I have to look it up myself all the time.

Your example is—yet again—extremely artificial. What are you trying to do/get from aesara.tensor.random.basic? Are you trying to create a random variable? If so, try at.random.* from the standard at alias instead. Here's an example:

import aesara.tensor as at

X_rv = at.random.normal(0, 1)

Seems pretty simple to me. Also, if you want to compare this with NumPy, they're awfully close:

import numpy as np

X = np.random.normal(0, 1)

What's the problem again?

Let's take this a little further and imagine that we wanted to get the class representing the RNG object. In Aesara, that's aesara.tensor.random.utils.RandomStream, but it's available from aesara.tensor.random.RandomStream. The NumPy equivalent is numpy.random._generator.Generator, and, in nearly direct accordance with Aesara, it's available as numpy.random.Generator; however, notice that it's not available at the top-level as numpy.Generator.

The first one is simply FUD, and it's not welcome here.

Not cool to accuse me of propaganda techniques that I even have to look up. I am contributing these suggestions for improvement with positive intent, but responses like that and categorical dismissal of my contributions (e.g. labeling "wontfix" before even waiting for a third opinion) really make me feel unwelcome here.

Claiming that you can't show Aesara to people without "frightening" them and claiming that people need "in-depth knowledge"/"to know a lot about the internal structure" of the library in order to import classes is neither true, positive, nor constructive. They're not the kind of "contributions" we want or need.

You're stretching some import inconveniences that haven't even been motivated by cogent user-level use-cases into threats against the usability of the library in general. No matter what your intentions are, this is neither helpful nor welcome.

The issue was labeled "wontfix" because I do not want someone to spend time implementing your very explicit all-top-level-imports recommendation/request, especially without a discussion that actually addresses the underlying concerns, requirements, alternatives, etc.

Furthermore, your claim of "categorical dismissal" is an outright fabrication, as evidenced by this very discussion. If your comments go further in these directions, they will be moderated.

[michaelosthege]

Your example is—yet again—extremely artificial. What are you trying to do/get from aesara.tensor.random.basic? Are you trying to create a random variable? If so, try at.random.* from the standard at alias instead. Here's an example:

Thank you for this example! I was not aware of this import route and this is the first time someone has pointed out that I was using an overcomplicated import route all the time!!

As for the other imports I do think there's room for improvement so third-party libraries such as PyMC3 that use Aesara can depend on robust import routes that are less verbose than, for example these.

W.r.t. the other things I'm simply trying to communicate my experience in using the library as well as how I perceive this discussion. I think the observation that I experience so much friction with the API and the discussion should be a warning sign to think about API and discussion friction.

But I'm not going to continue bothering you with any of this. Ping me if you want my input on something.

[ricardoV94]

Also this is a more broad one, but the "basic" submodules feel a bit weird (on the user side, not on the developer side).

For example, aesara.graph.basic feels one level too much. Why wouldn't aesara.graph give access to the "basic" objects directly (or at least the most common ones used by users)?

[twiecki]

Agreed.

[brandonwillard]

Yes, and, from my first comment:

Some of the basic classes could be exposed at higher package levels, ...

Also:

At most, Variable could be at the top level and TensorVariable at aesara.tensor.

Some background:

After refactoring and removing circular imports, the plan was always to re-expose some basic objects at more convenient package levels.

Aside from the cases retained from Theano, you can see this plan unfolding in even the recently developed sub-packages like aesara.tensor.random, aesara.link.numba, aesara.link.jax, where objects are exposed at sub-package level (i.e. __init__.py) for both user and developer convenience.

This idea/subject itself is a standard software library development consideration, so there's not much more to say about it. The real work is in the details, where one of the most important distinctions to make is whether or not one's considering a change for user or developer purposes.

In other words, the question has never been about whether or not we should/could expose things at (sub-)package level; it's about exactly what we should expose at a higher level, why we should do it, and at exactly which level we should expose those things.

When it comes to user-level considerations, there are certain things we are obligated to do (e.g. make objects available in a way that mirrors NumPy's user interface). That's easier to deal with, because it's a stricter requirement.

For developers/internally, we can obviously do whatever we want, but we should do that without re-introducing unnecessary, confusing, and restrictive circular import dependencies. Sometimes such things aren't introduced directly, but arise over time from a combination of poor package layout/design choices and an over-use of indirect imports. To avoid those situations, project-wide self-restraint helps a lot (e.g. limit unjustified imports in __init__, import from modules in which an object is defined, etc.).

More generally, if we act like greedy, local optimizers and make changes that simply sound good because there's an identifiable convenience or two behind them, then we'll quickly end up in "local minima" that are difficult to escape. As project managers, if these considerations are not at the forefront of our discussions, then we're not doing our jobs very well.

[brandonwillard]

Also this is a more broad one, but the "basic" submodules feel a bit weird (on the user side, not on the developer side).

To summarize and contextualize the above statements, let's start with the following: What is the user side of aesara.graph.basic?

As I mentioned in my previous comments, almost none of the classes in aesara.graph.basic can—or should—be used directly, and especially not by a standard user of the library (i.e. someone who wants to construct and compile graphs for existing, NumPy-like operations).

[ricardoV94]

Do PyMC3 developers count as Aesara users?

[brandonwillard]

PyMC3 involves a considerable amount of Aesara/Theano development, so no.

Here are some rules-of-thumb:

• If you're creating custom Ops, then you're definitely doing Aesara development.
• If you're programmatically constructing Aesara graphs, then you're very likely doing Aesara-level development, especially when it involves the inspection and/or manipulation of the objects produced by Aesara.