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

'Factorize' fill aesthetic #191

Closed
funnell opened this issue Jan 30, 2014 · 4 comments
Closed

'Factorize' fill aesthetic #191

funnell opened this issue Jan 30, 2014 · 4 comments

Comments

@funnell
Copy link

funnell commented Jan 30, 2014

When the fill aesthetic is set, it should be handled in a manner similar to colour, size, etc.

@jankatins
Copy link
Contributor

@funnell Want to do a PR? The code is in ggplot/components/...

@funnell
Copy link
Author

funnell commented Feb 1, 2014

I'll have a look

jankatins added a commit to jankatins/ggplot that referenced this issue Feb 6, 2014
Data transformation in aes (`aes(x="np.log(column)")' now uses
patsy.eval.EvalEnvironment. This should enable things like
`np.log(column)`.

Closes: yhat#188

The changes also let some bug in the current unittests show up:
setting a aes mapping (`aes(fill=True)`) was considered equivalent
to setting this values in the geom `geom_density(fill=True)`). Now
this will result in the same weired result as in ggplot (if we
would have already implemented fill... -> yhat#191). The affected
unittests (test_basic.py, test_readme_examples.py) were changed.

Also implement `__depcopy__()` for `aes` and ´ggplot` to not
deepcopy the needed eval environment as deepcopy failed with
the above change. ggplot deepcopy now does *not* copy the
dataframe, so this should result in some speedups. Also adjusted
the unittest in test_geom.py to fit this new model.

Closes: yhat#145

Added unittests (test_ggplot_internals.py) to make sure that the
original data is not changed and also that no data is changed
after a geom addition.
@jankatins
Copy link
Contributor

See #199 for an example what currently does not work with fill :-(

@jankatins
Copy link
Contributor

What is needed:

  • a new ggplot/components/fill.py with a assign_fill, which adds a new legend with type 'fill' (gg.add_to_legend(...))
  • assign_fill needs to be added in ggplot/components/__init__.py
  • the corresponding make_line_key() function and a addition to legend_viz in ggplot/components/legend.py

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

3 participants