Ignite: Master engine docs missing something

[ydcjeff]

🐛 Bug description

Latest master engine docs is missing some classes under Engine class, I believe.
Also CallableEventWithFilter link is not working.

[vfdev-5]

@trsvchn could you please take a look ?

[trsvchn]

Yes, some classes are really missed, but CallableEventWithFilter broken link is expected, since CallableEventWithFilter is not listed under ignite.engine.events section with autoclass.

I will fix that!

[ydcjeff]

Also @vfdev-5 instead of going straight down to the docs (eg to Engine class docs when you click Engine)
would it be better to have separate page Engine like pytorch did with Module, Sequential like here

I prefer to read like that since I am used to pytorch docs
What do you think?

[vfdev-5]

I will fix that!

@trsvchn thanks !

Also @vfdev-5 instead of going straight down to the docs (eg to Engine class docs when you click Engine)
would it be better to have separate page Engine like pytorch did with Module, Sequential like here

Well, this type of docs in pytorch is rather new since 1.6.0 and seems OK as they have a lot of various classes in nn module.
In our case spliting into multiple pages will lead to almost empty page of ignite.egine with 7 items...
The main point of using autoclass was to provide automatically TOC for the module. Then Taras made it nicer with pytorch-like TOC. The white space between Engine and CallableEventWithFilter is actually intended (see https://pytorch.org/ignite/master/_sources/engine.rst.txt). Probably, to make it neat we can add some text around ... Thoughts ?

[trsvchn]

yes here with engine we have a little tricky situation, it consists of submodules: events, deterministic and engine, and some classes of these submodules are declared as public under engine module see egine/__ini__.py __all__ . Thus, to avoid duplication I auto-listed each submodule individually and that's why we have white spaces Engine and CallableEventWithFilter etc because they are from different submodules. AND in addition when auto-listing engine.engine our tool not so smart to differentiate imported classes from submodules and that's breaks imports paths.

[ydcjeff]

I see! Yep, I have played around a bit and found out generating each sub page for each class and function doesn't look good with small docstring.

Probably, to make it neat we can add some text around ... Thoughts ?

How about we add title with api the end users have to use like ignite.engine.events.
Under that, Events, EventEnum, ... will go in. Sounds good? @vfdev-5

[vfdev-5]

How about we add title with api the end users have to use like ignite.engine.events.
Under that, Events, EventEnum, ... will go in. Sounds good? @vfdev-5

Yes, maybe. It depends how it will be rendered.

I was thinking to add text between submodules such that white spaces will be filled: https://github.com/pytorch/ignite/pull/1461#issuecomment-726802950

[ydcjeff]

how about that text be sub modules' names, so it's easier to recognize those classes are from this sub modules.

[vfdev-5]

how about that text be sub modules' names, so it's easier to recognize those classes are from this sub modules.

OK, let me merge @trsvchn 's PR and you can draft another one with your suggestion and we can iterate over it. How does sound for you ?

[ydcjeff]

Sounds good!

[vfdev-5]

Ops, issues was accidentally closed by merging Taras' PR