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

Cleaning up Imagen #46

Open
jlstevens opened this issue Apr 28, 2015 · 3 comments
Open

Cleaning up Imagen #46

jlstevens opened this issue Apr 28, 2015 · 3 comments
Milestone

Comments

@jlstevens
Copy link
Contributor

I like the core of ImaGen a lot: I think pattern generators a good idea and very useful in practice. Then there is the rest of the code that I don't like, either because the code quality is poor or because the code is addressing problems that I don't feel should be part of ImaGen.

It would be very nice to have a small, clean library called ImaGen instead of what we have now: a nice core surrounded by confusing, unused or poorly implemented code. As I know it is too late to remove this cruft as we have released Imagen 2.0 (note: cruft is a relative term to the core goal of the library!) here is what I would like to see:

pip install imagen

Includes the core of imagen:

  • __init__.py
  • patterngenerator.py: Mostly ok, needs formatting fixes.
  • patternfn.py: Should be a collection of staticmethods on a class ArrayGenerators
  • random.py: Useful!
  • image.py: Useful!
  • transferfn.py: Useful! Although some reorganization needed here.

This list includes everything I think should be on the master branch. Everything else should do on a branch called `'extras'`` that can be installed as follows:

pip install imagen[extras]

The 'extras' branch is where everything else goes:

  • audio.py
  • colorspaces.py
  • deprecated.py
  • patterncoordinator.py

I think five nice and clean files in core and four additional files in extras (for more specialist needs) would be a nice arrangement. This organization would encourage me to clean up core:

  • Move array functions to staticmethods on some class (already mentioned)
  • Remove comments and turn them into Issues as appropriate.
  • Docstring and formatting fixes.

Then when you run pip install imagen you get a nice, clean little library with a clear focus without all the other distracting code around it!

Just to be clear, I am not intending to do any of this now but it would be nice if we could agree on a plan...

@jbednar
Copy link
Contributor

jbednar commented May 5, 2015

This plan sounds unwieldy to me, and it seems more useful to spend the time it would take to set it up to instead polish up those files to be more usable and cleaner in implementation:

Deprecated.py should just be eliminated at some point, so there's not much reason to set up a special mechanism for it. PatternCoordinator solves a real problem of generating sets of patterns with relationships between them, and so I'd rather clean it up than move it out of the way. Colorspaces should either be cleaned up, if it needs to be here, or moved out, if it doesn't; the halfway step of putting it on a branch doesn't seem like it would help avoid any maintenance issues. Audio.py seems less clear; it's important to have some basic support for audio inputs (whether or not they are often used in practice), but there's also some more-detailed components in there that are more dubious even though they do at least refer to published work. Again, having a separate branch for audio.py doesn't seem like it accomplishes anything other than making sure that it will become unimportable at some point.

What about just having an extras submodule for the extra bits? That seems like much less work than switching between branches. Unfortunately, all of our .ty files currently depend on deprecated.py, and we'd want to remove that dependency between doing anything with that file...

@jlstevens
Copy link
Contributor Author

Ok. Imagen will stay a mess then because no one is working on it.

@jbednar
Copy link
Contributor

jbednar commented May 5, 2015

Sigh. :-/

@jbednar jbednar added this to the Wishlist milestone Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants