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

Enable adding new convertion functions #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Lande24
Copy link

@Lande24 Lande24 commented Sep 6, 2015

Changed valid_conversions to be an instance variable so that it can be overridden in classes inheriting from Mutate. This will allow adding new convert functions simply by inheriting from the Mutate class.

@purbon
Copy link

purbon commented Sep 7, 2015

Hi,
thanks a lot for your contribution @Lande24. In order to move forward with your contribution you will need to sign the CLA, more info at -> https://www.elastic.co/contributor-agreement

On the other side, i'm wondering what drove you to propose this change? for now mutations are operations written inside the same mutate class. Do you have a concrete improvement proposal that would use this change?

Thanks a lot,

@Lande24
Copy link
Author

Lande24 commented Sep 16, 2015

Hi and thanks for your time, I signed the CLA as required.
The reason I proposed this change is that I have a very specific conversion that I need to make from one Geo format to another. It would be unfortunate if I had to create an entirely new filter just for that conversion. In addition, I might want to make different conversions on other fields in which case I would have to use two different filters, which is inconvenient.
Thanks again.

@jordansissel
Copy link
Contributor

I don't quite understand the goal of this change. Can you explain more? It sounds like you are subclassing the mutate filter? Can you help me understand why and what you are doing?

@Lande24
Copy link
Author

Lande24 commented Oct 1, 2015

I want to add a specific geo conversion from one geo format to another.
My intention is to subclass Mutate and just implement the conversion method I want (I want to convert to GeoJson, so I will implement the method convert_geojson).
So I essentially added my functionality with minimal code replication.

What my change does is it makes the valid_conversions variable a class variable so that I can change it in my subclass and add other valid conversions (such as GeoJson).

@Lande24
Copy link
Author

Lande24 commented Oct 1, 2015

When I said "class variable" I meant instance variable actually..

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants