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

Relative imports, RETURNN import_, or intended more stable? #2

Open
albertz opened this issue Jun 3, 2021 · 1 comment
Open

Relative imports, RETURNN import_, or intended more stable? #2

albertz opened this issue Jun 3, 2021 · 1 comment

Comments

@albertz
Copy link
Member

albertz commented Jun 3, 2021

Relative imports can be difficult to read in some cases when you need to go up a couple of packages.
Sth like from returnn_common... import ... can look nicer and cleaner in some cases.

However, this currently is not possible when it should work with the RETURNN import_ mechanism.

Maybe we can somehow fix the RETURNN import_ mechanism to make this possible, although I'm afraid this would be hacky, and I don't see a simple solution currently. (If we want to go this route, let's open a separate issue in the RETURNN repo about this.)

The question is also whether we need import_ here. import_ is intended for unstable code which can easily break. But in e.g. i6_experiments we handle this differently:

  • Common code (in common subdir in i6_experiments) is supposed to be relatively stable and not break too much, or tries to be compatible for older setups. (It's not really settled, and maybe less strict than RETURNN itself, or i6_core, but still.)
    This corresponds to the code we have here directly, e.g. in models, etc.

  • Users code (in users subdir in i6-exp) can be as unstable as someone wants. There are no guarantees.
    We have the same here, although naming might be different (we would not have a users sub-dir but instead custom modules with some postfix, like conformer_chris.py or so).

So maybe this is fine, and then we do not need import_. Instead, we can handle this repo in a similar way as i6_experiments, clone it into the recipe subdir for Sisyphus, and make it also available for RETURNN configs.

@JackTemaki
Copy link
Contributor

Instead, we can handle this repo in a similar way as i6_experiments, clone it into the recipe subdir for Sisyphus.

I would suggest to go with this way, as this will be the most convenient and obvious solution for Sisyphus users.

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

No branches or pull requests

2 participants