You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Improvement Description
Reduce coupling by grabbing View type annotations from wrapped function signatures and using appropriate transformers instead of hard-coding acceptable view_types in the decorator.
Current Behavior
At present, _disallow_empty_tables is tightly coupled to the table view types specified by the functions it wraps. If a new view type is used for tables in a function, the decorator will need to be refactored to explicitly allow that view type.
Proposed Behavior
Introspect view type annotations in wrapped function signatures. Use get_transformer() to get a transformer to biom.Table. If none exists, fail on invalid view type/missing transformer. Else, transform to biom.Table and check is_empty() as currently.
Questions
Should error messages here be developer-facing (e.g. no transformer exists from View type to biom.Table) or user facing as currently implemented?
Comments
Unless we see an increase in the number of possible table View types, this 'improvement' might just be over-engineering.
The text was updated successfully, but these errors were encountered:
Improvement Description
Reduce coupling by grabbing View type annotations from wrapped function signatures and using appropriate transformers instead of hard-coding acceptable view_types in the decorator.
Current Behavior
At present,
_disallow_empty_tables
is tightly coupled to thetable
view types specified by the functions it wraps. If a new view type is used for tables in a function, the decorator will need to be refactored to explicitly allow that view type.Proposed Behavior
Introspect view type annotations in wrapped function signatures. Use
get_transformer()
to get a transformer tobiom.Table
. If none exists, fail on invalid view type/missing transformer. Else, transform to biom.Table and check is_empty() as currently.Questions
no transformer exists from View type to biom.Table
) or user facing as currently implemented?Comments
Unless we see an increase in the number of possible
table
View types, this 'improvement' might just be over-engineering.The text was updated successfully, but these errors were encountered: