Look at the full collection of Exchange classes

[shreve]

In nbgrader.apps.baseapp.NbGrader.all_configurable_classes, there was a subtle problem in the way it was deciding whether to include classes from exchange.default which meant that none were ever included.

The same work to do this check was being done 5 different times in unique blocks of code, one of which being incorrectly implemented. To avoid this type of mistake in the future, I added an inline function to do that check so it's done in a uniform way every time.

Fixes #1724

[github-actions]

👈 Launch a Binder on branch shreve/nbgrader/fix-configurable-class-list

[shreve]

There is a kind of interesting question about the right way to do this. The way it exists in the PR, we add all the classes from exchange.abc, then all the classes from exchange.default, so they show up separately in the generated config.

If instead we just included all the classes from exchange.default, the traitlets Application._classes_with_config_traits method will pull in each of the parent abc classes and add them to the list so that they are interspersed and next to each other in the config file. That would look like the code below.

It's worth noting that this adds some kind of confusing noise to the generated config file because of the way traitlets works.

#------------------------------------------------------------------------------
# Exchange(LoggingConfigurable) configuration
#------------------------------------------------------------------------------
## Local path for storing student assignments.  Defaults to '.' which is normally
#  Jupyter's root_dir.
#  Default: '.'
# c.Exchange.assignment_dir = '.'

## Format string for timestamps
#  Default: '%Y-%m-%d %H:%M:%S.%f %Z'
# c.Exchange.timestamp_format = '%Y-%m-%d %H:%M:%S.%f %Z'

## Timezone for recording timestamps
#  Default: 'UTC'
# c.Exchange.timezone = 'UTC'

#------------------------------------------------------------------------------
# Exchange(Exchange) configuration
#------------------------------------------------------------------------------
## 
#  See also: Exchange.assignment_dir
# c.Exchange.assignment_dir = '.'

## Local cache directory for nbgrader submit and nbgrader list. Defaults to
#  $JUPYTER_DATA_DIR/nbgrader_cache
#  Default: ''
# c.Exchange.cache = ''

## Whether the path for fetching/submitting  assignments should be prefixed with
#  the course name. If this is `False`, then the path will be something like
#  `./ps1`. If this is `True`, then the path will be something like
#  `./course123/ps1`.
#  Default: False
# c.Exchange.path_includes_course = False

## The nbgrader exchange directory writable to everyone. MUST be preexisting.
#  Default: '/usr/local/share/nbgrader/exchange'
# c.Exchange.root = '/usr/local/share/nbgrader/exchange'

## Format string for timestamps
#  See also: Exchange.timestamp_format
# c.Exchange.timestamp_format = '%Y-%m-%d %H:%M:%S.%f %Z'

## Timezone for recording timestamps
#  See also: Exchange.timezone
# c.Exchange.timezone = 'UTC'

[shreve]

Rebased to get the windows timeout fix

[perllaghu]

If instead we just included all the classes from exchange.default, the traitlets Application._classes_with_config_traits method will pull in each of the parent abc classes and add them to the list so that they are interspersed and next to each other in the config file. That would look like the code below.

Would this pull in all the classes from exchange.myExchange - which is what the whole exchange.abc thing is there to enable?

[genuine question - I don't know nearly enough about traitlets, configuration, etc to know...]

[brichet]

Thanks @shreve, I have one comment about this PR.

[brichet]

Suggested change

	_collect_configurables(exchange.default)
	_collect_configurables(exchange.default.exchange)

Do you think we need to catch all the default classes traits if they are similar to the abc ones ?
Since exchange is the only default class with dedicated traits, maybe it can be specified when calling the _collect_configurables function.
This should avoid duplication for most of the exchange classes, but I'm not sure it will not break anything about the functionalities, I don't know well traitlets.

[shreve]

After digging into this more, I think the duplication is just the nature of the beast. I have a new proposal to make the distinction more clear, which is to actually rename the abc classes.

When they're used in the nbgrader codebase, we already do from exchange.abc import Exchange as ABCExchange, and if you search GitHub, so does everyone else: https://github.com/search?q=from+nbgrader.exchange+import+&type=code

By renaming the actual class, we can get rid of these import aliases without touching the implementation, and create distinction in the generated config file.

[shreve]

@perllaghu

Would this pull in all the classes from exchange.myExchange - which is what the whole exchange.abc thing is there to enable?

No, traitlets only traverses up the inheritance chain, and a custom exchange would be down it.

We could instead ask the exchange factory for the configured classes, but this method is used for the task of generating a config file, so it's kind of a chicken and egg situation.

[shreve]

As mentioned in my reply, I want to rename the classes in nbgrader.exchange.abc. This is what the config would look like after this change for ExchangeList, for example.

Traitlets doesn't offer any way to configure the way this config is generated, so the duplication is inescapable for now. The best thing to do IMO is lean into it and make the different classes distinguishable.

#------------------------------------------------------------------------------
# ABCExchange(LoggingConfigurable) configuration
#------------------------------------------------------------------------------
## Local path for storing student assignments.  Defaults to '.' which is normally
#  Jupyter's root_dir.
#  Default: '.'
# c.ABCExchange.assignment_dir = '.'

## Format string for timestamps
#  Default: '%Y-%m-%d %H:%M:%S.%f %Z'
# c.ABCExchange.timestamp_format = '%Y-%m-%d %H:%M:%S.%f %Z'

## Timezone for recording timestamps
#  Default: 'UTC'
# c.ABCExchange.timezone = 'UTC'

#------------------------------------------------------------------------------
# Exchange(ABCExchange) configuration
#------------------------------------------------------------------------------
## 
#  See also: ABCExchange.assignment_dir
# c.Exchange.assignment_dir = '.'

## Local cache directory for nbgrader submit and nbgrader list. Defaults to
#  $JUPYTER_DATA_DIR/nbgrader_cache
#  Default: ''
# c.Exchange.cache = ''

## Whether the path for fetching/submitting  assignments should be prefixed with
#  the course name. If this is `False`, then the path will be something like
#  `./ps1`. If this is `True`, then the path will be something like
#  `./course123/ps1`.
#  Default: False
# c.Exchange.path_includes_course = False

## The nbgrader exchange directory writable to everyone. MUST be preexisting.
#  Default: '/usr/local/share/nbgrader/exchange'
# c.Exchange.root = '/usr/local/share/nbgrader/exchange'

## Format string for timestamps
#  See also: ABCExchange.timestamp_format
# c.Exchange.timestamp_format = '%Y-%m-%d %H:%M:%S.%f %Z'

## Timezone for recording timestamps
#  See also: ABCExchange.timezone
# c.Exchange.timezone = 'UTC'

#------------------------------------------------------------------------------
# ABCExchangeList(ABCExchange) configuration
#------------------------------------------------------------------------------
## 
#  See also: ABCExchange.assignment_dir
# c.ABCExchangeList.assignment_dir = '.'

## List assignments in submission cache.
#  Default: False
# c.ABCExchangeList.cached = False

## List inbound files rather than outbound.
#  Default: False
# c.ABCExchangeList.inbound = False

## Remove, rather than list files.
#  Default: False
# c.ABCExchangeList.remove = False

## Format string for timestamps
#  See also: ABCExchange.timestamp_format
# c.ABCExchangeList.timestamp_format = '%Y-%m-%d %H:%M:%S.%f %Z'

## Timezone for recording timestamps
#  See also: ABCExchange.timezone
# c.ABCExchangeList.timezone = 'UTC'

#------------------------------------------------------------------------------
# ExchangeList(ABCExchangeList, Exchange) configuration
#------------------------------------------------------------------------------
## 
#  See also: ABCExchange.assignment_dir
# c.ExchangeList.assignment_dir = '.'

## Local cache directory for nbgrader submit and nbgrader list. Defaults to
#  $JUPYTER_DATA_DIR/nbgrader_cache
#  See also: Exchange.cache
# c.ExchangeList.cache = ''

## List assignments in submission cache.
#  See also: ABCExchangeList.cached
# c.ExchangeList.cached = False

## List inbound files rather than outbound.
#  See also: ABCExchangeList.inbound
# c.ExchangeList.inbound = False

## 
#  See also: Exchange.path_includes_course
# c.ExchangeList.path_includes_course = False

## Remove, rather than list files.
#  See also: ABCExchangeList.remove
# c.ExchangeList.remove = False

## The nbgrader exchange directory writable to everyone. MUST be preexisting.
#  See also: Exchange.root
# c.ExchangeList.root = '/usr/local/share/nbgrader/exchange'

## Format string for timestamps
#  See also: ABCExchange.timestamp_format
# c.ExchangeList.timestamp_format = '%Y-%m-%d %H:%M:%S.%f %Z'

## Timezone for recording timestamps
#  See also: ABCExchange.timezone
# c.ExchangeList.timezone = 'UTC'