Document rules for conditional imports

[hvy]

Conditional imports. in our case, optional 3rd party dependencies, are not covered by PEP-8 and gives us some flexibility. At the same time, this could cause ambiguity. How about agreeing on some consensus and document it where and how they should appear in the code?

Conditional imports in Optuna look something like the following.

import numpy 

import optuna

# The conditional import comes in a separate island after standard imports.
try:
    import cma
    _available = True
except ImportError:
    _available = False

Note that we additionally often assign a "available" variable depending on the condition.
They frequently appear under visualization/* and integration/*.

[hvy]

One solution would be to simply not define any rules at all, as it might be difficult to cover all cases. For instance where the conditions depend on a different import, e.g. https://github.com/optuna/optuna/blob/master/optuna/integration/cma.py#L26-L35.

[hvy]

For simple cases such as the one in the description, however, I'm wondering if the following isn't cleaner. I.e. import at the PEP-8 suggested location, surround with try, except and allow a single assignment.

try:
    import cma
except ImportError:
    cma = None
import numpy 

import optuna

This does obviously not catch the error object but I believe that the reason is obvious in most of the cases for ImportErrors.

[github-actions]

This issue has not seen any recent activity.

[himkt]

@hvy

It’s reasonable for me to put a conditioned import including a single assignment on the bottom of imports.
(I think a conditioned import including a variable assignment should appear only in integration module.)

And, it might be out of scope in this PR, I think putting a conditioned import without a variable assignment below local library specific imports is natural since it may use methods in a local library. (i.e. optuna.type_checking.TYPE_CHECKING in https://github.com/optuna/optuna/blob/master/optuna/testing/distribution.py#L4)

The following is just my opinion:

import math  # STL

import numpy  # Third-party

import optuna  # Local

# Conditioned import without variable assignment
if optuna.type_checking.TYPE_CHECKING:
    import typing

# Conditioned import with variable assignment
try:
    import cma
except ImportError:
    cma = None

I want to continue this discussion.

[hvy]

Thanks, that sounds good. Clearly separating non-trivial (conditioned) imports seems to be more readable too.

On a second thought though, I'm wondering if just mentioning that "conditional or non-trivial imports should come after standard (std-3rdparty-internal) import" won't suffice for coding style guidelines. For instance, the TYPE_CHECKING conditions should be less frequent now that we're avoiding comment style type hints, and giving more details might just cover less cases as imports could in practice come with any code.

[github-actions]

This issue has not seen any recent activity.

[hvy]

Leaving a note. This issue is somewhat alleviated by this PR #1315.

[himkt]

Thank you for mentioning your update and sorry that I haven't made any updates for a long time.

I think this issue can be closed with two factors: first, @hvy will introduce a module for optional importing. This PR would be able to be a drop-in replacement for imports of third party libraries (e.g. https://github.com/optuna/optuna/blob/master/optuna/integration/chainer.py#L4-L15). Second, we now don't have to do availability checkings for type checking anymore (e.g. #1270).

@hvy
So, I'll close this issue if you agree with me.
What do you think?

[hvy]

Yes, I think it's fine to close this ticket. It's non-trivial to establish proper guidelines and the merits aren't that significant.