-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Closed
Description
See below for a small collection of nitpicks; I suppose those will be adressed before the first release, but wanted to write them down somewhere:
- Documentation seems to be in a different format so will need to be refactored to work with the
doc-builder - The
loggingclass still has mentions to🤗 Transformers - You have some
tqdminstances in schedulers, but they're not using the utility defined inloggingin order to toggle bars on and off - Some copyrights have wrong attribution (e.g.,
configuration_utilswhich mentions NVIDIA) - In some places you define a global variable for configuratio name (e.g.,
SCHEDULER_CONFIG_NAME = "scheduler_config.json"), in other places you define them as a string (config_name = "model_index.json", inDiffusionPipeline) - We have some conventions in
transformersthat IMO make sense for this library as well, wondering if it is an oversight or a decision to do differently. Those that I can see are: single letter variables, message-less asserts - The
register_modulesdoesn't seem to play well with code completion tools - The CLIP model is entirely copy/pasted from
transformers, is the goal to upstream the change at some point or to keep it separate in that repo?
patrickvonplaten, anton-l and patil-suraj
Metadata
Metadata
Assignees
Labels
No labels