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

Fvtk cleanup #378

Merged
merged 6 commits into from
Jun 11, 2014
Merged

Fvtk cleanup #378

merged 6 commits into from
Jun 11, 2014

Conversation

MrBago
Copy link
Contributor

@MrBago MrBago commented Jun 4, 2014

  • I've cleaned up how colormaps are handled a little in dipy, and moved the actual colormap data information from the code into a json file. This makes the code cleaner and makes it easier to handle matplotlib colormaps. All matplotlib colormaps can be used in any part of fvtk if the user has matplotlib installed
  • I've replaces all the if/elif/elif calls in fvtk with a new function
  • Added tests, and fixed a bug I discovered while writing said tests, for colormaps.

This should supplant #376 and #377 I believe.

@MrBago
Copy link
Contributor Author

MrBago commented Jun 4, 2014

So I used indent=2 when i made the json file because I thought it would be more readable, but it still doesn't seem all that readable. Should I replace the long json file with a smushed json stream instead? Pickle file could work too.

@arokem
Copy link
Contributor

arokem commented Jun 4, 2014

You mean to suggest that it's better to solve something once properly, than to solve it many times half-way?

:-)

@arokem
Copy link
Contributor

arokem commented Jun 4, 2014

Thanks for doing this!

@arokem
Copy link
Contributor

arokem commented Jun 4, 2014

I think the json file looks ok

name of the colourmap
name : str.
name of the colormap. Currently implemented: 'jet', 'blues',
'accent', 'bone' and matplotlib colormaps if you have mapplotlib
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapplotlib?

arokem added a commit that referenced this pull request Jun 11, 2014
@arokem arokem merged commit fe69c87 into dipy:master Jun 11, 2014
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

Successfully merging this pull request may close these issues.

3 participants