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

rename call methods to transform and postprocess #4044

Merged
merged 5 commits into from Aug 20, 2013

Conversation

ivanov
Copy link
Member

@ivanov ivanov commented Aug 15, 2013

  • The call methods for nbconvert transformers has been renamed to
    transform.

  • The call methods of nbconvert post-processsors have been renamed to
    postprocess.

    pinging @Carreau and @jdrefer, who probably knows who'd be affected by
    this change... maybe @damianavila, @jakevdp

@jakevdp
Copy link
Contributor

jakevdp commented Aug 15, 2013

Thanks for the ping. This is not a 1.0 bug fix, but a 2.0 feature, correct?

@damianavila
Copy link
Member

No problems here... ;-)

@minrk
Copy link
Member

minrk commented Aug 15, 2013

yes, should be 2.0

@Carreau
Copy link
Member

Carreau commented Aug 15, 2013

+1.

Want to keep call as deprecated alias ?

@ivanov
Copy link
Member Author

ivanov commented Aug 15, 2013

@Carreau I consulted with Min about that as I was writing the PR, and since we said nbconvert is alpha-tech preview, we don't need to - we're allowed to change things there at will (it would be kind of a nightmare to keep everything compatible with the code we have in 1.0, since so much more cleanup will go into nbconvert for 2.0

@Carreau
Copy link
Member

Carreau commented Aug 15, 2013

@ivanov ah, yes, you are right.

@minrk
Copy link
Member

minrk commented Aug 15, 2013

possible change: for symmetry / clarity, should we rename 'transformers' to 'preprocessors'? It seems more descriptive, and we have frequent confusion about the difference between transformers and filters (from those that aren't aware that filters are a jinja thing).

@ivanov
Copy link
Member Author

ivanov commented Aug 15, 2013

I'm +1 on the preprocessor renaming

@damianavila
Copy link
Member

yes, I agree...

@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

just to clarify, I'm adding to the PR a global search-replace of Transformer -> Preprocessor, yes?

@minrk
Copy link
Member

minrk commented Aug 16, 2013

Sure - also probably means change transform to process?

@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

yeap, ok, that's what I'm doing, update coming shortly

@Carreau
Copy link
Member

Carreau commented Aug 16, 2013

Note that this will conflict with #3747

@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

done with preprocessing, I'll send you a patch to fixup 3747, Matthias, if you'd like

@Carreau
Copy link
Member

Carreau commented Aug 16, 2013

No I can do it, just want to pint out.
You will still need to rebases this one.

@Carreau
Copy link
Member

Carreau commented Aug 16, 2013

have you lost some git-fu @ivanov ? you are doing weird stuff when you rebase.

@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

yeah, hang on, working on it

- The `call` methods for nbconvert transformers has been renamed to
  `transform`.
- The `call` methods of nbconvert post-processsors have been renamed to
  `postprocess`.

  pinging @Carreau and @jdrefer, who probably knows who'd be affected by
  this change...  maybe @damianavila, @jakevdp
@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

alright, how's that @Carreau

@Carreau
Copy link
Member

Carreau commented Aug 16, 2013

Better, travis still in progress.

@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

are you not hip enough for hipchat, or too hip?

@Carreau
Copy link
Member

Carreau commented Aug 16, 2013

This is starting to looks good. You have my green light to break master. When you'll push the green button, my EuroSciPy talk will no longer be compatible with master :-)

@ivanov
Copy link
Member Author

ivanov commented Aug 16, 2013

Matthias Bussonnier, on 2013-08-16 14:22, wrote:

When you'll push the green button, my EuroSciPy talk will no
longer be compatible with master :-)

is that because your talk is more than meets the eye?

@Carreau
Copy link
Member

Carreau commented Aug 16, 2013

Actually it's a 90min tutorial. I hope to have enought time to use nbconvert from CL and as python lib. Potentially defining custom transformer. Anyway I guess most people will run 1.0 from conda (I hope, as it is still not default in their repo), so I'll run 1.x too.

@minrk
Copy link
Member

minrk commented Aug 17, 2013

Working with nbconvert APIs in the tutorial is going to be tricky, as they are guaranteed to change (as illustrated by the fact that it will already be broken on master by the time of the tutorial. We will have to tread lightly, and not emphasize APIs.

@ellisonbg
Copy link
Member

Honestly, I don't know if I would even cover the nbconvert APIs right now
in a public setting...

On Fri, Aug 16, 2013 at 6:16 PM, Min RK notifications@github.com wrote:

Working with nbconvert APIs in the tutorial is going to be tricky, as they
are guaranteed to change (as illustrated by the fact that it will already
be broken on master by the time of the tutorial. We will have to tread
lightly, and not emphasize APIs.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4044#issuecomment-22802984
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@Carreau
Copy link
Member

Carreau commented Aug 17, 2013

Honestly, I don't know if I would even cover the nbconvert APIs right now
in a public setting...

Not in depth, but as soon as you speak architecture you use the name "Transfomer"
when speaking configuration you use c.Classname.traitname

All theses are touched by this PR. I try to avoid speaking of transformer/preprocesors as much as possible
But I still have to use the term here and there.

@ivanov
Copy link
Member Author

ivanov commented Aug 19, 2013

merging in a couple of hours unless I hear back otherwise

ivanov added a commit that referenced this pull request Aug 20, 2013
rename call methods to transform and postprocess
@ivanov ivanov merged commit 357b6fb into ipython:master Aug 20, 2013
@ivanov ivanov deleted the transform-not-call branch August 20, 2013 02:40
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
rename call methods to transform and postprocess
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.

None yet

6 participants