Skip to content

a basic implementation for Image[] and related functions - #363

Closed
ghost wants to merge 13 commits into
imathicsfrom
unknown repository
Closed

a basic implementation for Image[] and related functions#363
ghost wants to merge 13 commits into
imathicsfrom
unknown repository

Conversation

@ghost

@ghostghost commented Apr 29, 2016

Copy link
Copy Markdown

I'm submitting this to the iMathics branch as well now, since it doesn't seem to work on the regular Mathics notebook. It seems to be impossible to push HTML < img > tags into the regular Mathics notebook, which I'm doing in order to display the loaded images (Mathics gives an explicit error, saying that img nodes are an unknown node type). Jupyter doesn't have that limitation though. Needs scikit-image.

@sn6uv

Copy link
Copy Markdown
Member

I think you only want 515aa66 and on right?

Rather than making a new pull request to fix this I'd recommend something like:

git checkout upstream/imathics
git cherry-pick e796872..8a50e24
git branch -d image # delete old image branch
git checkout -b image # create a new image branch
git push -f -u origin image # force push the image branch over the old one

and the PR should update automatically.

@ghost

Copy link
Copy Markdown
Author

cherry pick worked like magic

@sn6uv

Copy link
Copy Markdown
Member

The current notebook only supports svg images. I think implementing this for the imathics branch only makes sense. (I'm currently trying to port the graphics/plot functionality over to using jupyter).

if n > 8192:
return evaluation.error('Colorize', 'toomany')

cmap = matplotlib.cm.get_cmap('hot', n)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might be worth splitting the matplotlib dependency out (separate to scikit-image) if this is the only place it's used.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I thought about this as well, but then scikit-image depends on matplotlib (and numpy and scipy), so it's probably not worth the effort:

https://github.com/scikit-image/scikit-image/blob/master/requirements.txt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point, I agree.

@sn6uv

sn6uv commented May 4, 2016

Copy link
Copy Markdown
Member

Lots of great functionality here!

I'm looking through this and woking on adding some error handling and docs/tests, see my Image branch.

@sn6uv

Copy link
Copy Markdown
Member

@poke1024 how do you want to handle collaborating on this? Should I open a PR against your branch with my changes?

@ghost

Copy link
Copy Markdown
Author

I'm actually not sure what's best. I'm actually quite comfortable with doing PRs on your branch (and calling yours the definite version).

@sn6uv

Copy link
Copy Markdown
Member

Okay, if you think that's easier. I'll close this and open a separate PR.

@sn6uvsn6uv closed this May 14, 2016
@sn6uvsn6uv mentioned this pull request May 14, 2016
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sn6uv@l1ebl