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

:members: and :inherited-members: implied for traits - a bit unexpected -> discussion of options for autoconfigurable #27

Open
consideRatio opened this issue Dec 8, 2022 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@consideRatio
Copy link
Member

consideRatio commented Dec 8, 2022

When a class' traits are documented with autodoc_traits, all of the traits are presented. That is quite unexpected in my mind, but also quite useful as it could often be that you want to present all configurable options while not presenting all functions etc.

Maybe we should keep doing this, just wanted to have an issue open about it for visibility for now.

@consideRatio consideRatio changed the title Placeholder issue :members: and :inherited-members: implied for traits - a bit unexpected Dec 8, 2022
@consideRatio consideRatio added the documentation Improvements or additions to documentation label Dec 8, 2022
@consideRatio
Copy link
Member Author

@minrk wrote in #28 (comment):

I think it's a valid goal to present the option to export only the configurable options, and not the Python API of a class. I'm not saying autoconfigurable without members is necessarily the right signature, but that output should be an option.

I agree, we should have a way to expose all traitlets members.

In my mind, we should provide a new configuration option, such as :config-members: to emit all traits with .config(True). This kind of API should probably allow us to distinguish between class specific configuration options and inherited config options as well.

@minrk
Copy link
Member

minrk commented Dec 14, 2022

I think :config-members: makes sense. I'm not certain what its inputs or defaults should be. I think we have a few questions to answer, that should clear it up:

  1. what does it look like to request all config on the current class, but not inherited config?
  2. what does it look like to request all config inherited by the current class (the default behavior before Add tests, add docstrings, and fix small bugs #28)?
  3. what does it look like to include only specific (or all minus specific) config members, a la the existing :members: or :exclude-members:?
  4. what does autoconfigurable: Class with no options given do by default (presumably it should be different from autoclass: Class for the same class. I'd say probably include the 'default' of all inherited or just current class config, with a slight inclination toward preserving current behavior unless there's a good reason to do something different).

Do we also need to think about non-config trait members, or leave that to the regular autoclass behavior with members, etc.? It seems like the existing behavior on autoclass with the trait-documenter works for that?

@consideRatio
Copy link
Member Author

consideRatio commented Dec 14, 2022

  1. what does it look like to request all config inherited by the current class (the default behavior before Add tests, add docstrings, and fix small bugs #28)?

It remains the default, its not changed! I'm updating the docs example to clarify that right now!

I'll reflect further later on other notes!

@consideRatio
Copy link
Member Author

consideRatio commented Dec 15, 2022

  1. what does it look like to request all config on the current class, but not inherited config?
  • I suggest a boolean :no-inherited-config-members: option to resolve this is we want this feature, but we currently don't support it.

  1. what does it look like to include only specific (or all minus specific) config members, a la the existing :members: or :exclude-members:?
  • Following Add tests, add docstrings, and fix small bugs #28, it is possible to use :exclude-members: to exclude config members.
  • We currently don't provide an option to specify specific config members to list. If the feature is merited to introduce I suggest we introduce :config-members: to provide a list of such config members. Where leaving it empty should then mean the same as the default behavior, which is to include all config members.

  1. what does autoconfigurable: Class with no options given do by default (presumably it should be different from autoclass: Class for the same class. I'd say probably include the 'default' of all inherited or just current class config, with a slight inclination toward preserving current behavior unless there's a good reason to do something different).
  • Agree, let's preserve the default behavior. The default behavior is to present all traits, but no non-traits members.

Summary

Possible new features, as options to the autoconfigurable directive.

  • :no-inherited-config-members:, to omit inherited config members
  • :config-members: <list>, to specify only specific config members to list

Existing functionality (after #28):

  • :exclude-members:, also functional for config members

@consideRatio consideRatio changed the title :members: and :inherited-members: implied for traits - a bit unexpected :members: and :inherited-members: implied for traits - a bit unexpected -> discussion of options for autoconfigurable Dec 15, 2022
@minrk
Copy link
Member

minrk commented Dec 15, 2022

Great summary!

I suggest a boolean :no-inherited-config-members: option to resolve this is we want this feature, but we currently don't support it.

👍 I think folks don't usually want to exclude inherited config, which may be why nobody's asked for this. No pressure to implement it, but good to document that we can't do it now, and know what it should look like if we do. It may make sense to stop at a particular class, rather than a bool.

The default behavior is to present all traits, but no non-traits members.

to be precise, all configurable traits, not all traits. I think there's still room for improving how we treat non-config trait members.

We currently don't provide an option to specify specific config members to list.

Why doesn't :members: work for this, if :exclude-members: does?

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

No branches or pull requests

2 participants