Skip to content

Commit eb8b274

Browse files
committed
Look at the correct collection of Exchange classes
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.
1 parent 7bbe818 commit eb8b274

File tree

2 files changed

+15
-21
lines changed

2 files changed

+15
-21
lines changed

nbgrader/apps/baseapp.py

+14-21
Original file line numberDiff line numberDiff line change
@@ -149,35 +149,28 @@ def all_configurable_classes(self) -> TypingList[MetaHasTraits]:
149149
if len(app.class_traits(config=True)) > 0:
150150
classes.append(app)
151151

152+
def _collect_configurables(module):
153+
"""Return a list of all configurable classes from a module"""
154+
155+
for name in module.__all__:
156+
cls = getattr(module, name)
157+
if hasattr(cls, "class_traits") and cls.class_traits(config=True):
158+
classes.append(cls)
159+
152160
# include plugins that have configurable options
153-
for pg_name in plugins.__all__:
154-
pg = getattr(plugins, pg_name)
155-
if pg.class_traits(config=True):
156-
classes.append(pg)
161+
_collect_configurables(plugins)
157162

158163
# include all preprocessors that have configurable options
159-
for pp_name in preprocessors.__all__:
160-
pp = getattr(preprocessors, pp_name)
161-
if len(pp.class_traits(config=True)) > 0:
162-
classes.append(pp)
164+
_collect_configurables(preprocessors)
163165

164-
# include all the exchange actions
165-
for ex_name in exchange.__all__:
166-
ex = getattr(exchange, ex_name)
167-
if hasattr(ex, "class_traits") and ex.class_traits(config=True):
168-
classes.append(ex)
166+
# include all the abstract exchange actions
167+
_collect_configurables(exchange)
169168

170169
# include all the default exchange actions
171-
for ex_name in exchange.default.__all__:
172-
ex = getattr(exchange, ex_name)
173-
if hasattr(ex, "class_traits") and ex.class_traits(config=True):
174-
classes.append(ex)
170+
_collect_configurables(exchange.default)
175171

176172
# include all the converters
177-
for ex_name in converters.__all__:
178-
ex = getattr(converters, ex_name)
179-
if hasattr(ex, "class_traits") and ex.class_traits(config=True):
180-
classes.append(ex)
173+
_collect_configurables(converters)
181174

182175
return classes
183176

nbgrader/exchange/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from nbgrader.exchange.abc import (Exchange, ExchangeError, ExchangeCollect, ExchangeFetch, ExchangeFetchAssignment,
33
ExchangeFetchFeedback, ExchangeList, ExchangeReleaseAssignment, ExchangeRelease,
44
ExchangeReleaseFeedback, ExchangeSubmit, ExchangeReleaseFeedback)
5+
from nbgrader.exchange import default
56
from .exchange_factory import ExchangeFactory
67

78
__all__ = [

0 commit comments

Comments
 (0)