GoogleCloudPlatform / dataflow-sample-applications

Apache License 2.0
126 stars 58 forks source link

A few question on how to contribute #42

Closed danp11 closed 2 years ago

danp11 commented 3 years ago

Hi,

In the source code one can see a few Todos and I wonder if you have an internal list of tickets that should be done for the java-timeseries repo?

I'd be happy to also code :-) but don't know where do start. Those "TS- tickets" that you refer to in the PR's, do you have a list of those for me/us to start with?

I would like to implement M/-VWAP and thought it we would be a nice addition to model classes for Price, Volume, Turnover etc. Not sure if AutoValue or Proto would be the the best fit though.

Any inputs on above thought would be appreciated. Thanks,

/Dan

rezarokni commented 3 years ago

Hi Dan,

First of all thank you for your interest and more importantly your offer to contribute.

We are still working out the details on the best path to externalise the todo list, but I think adding new metrics is a great place to start!

For now, we can make use of issues to track contribution work. Perhaps add an issue with the M/-VWAP as the title, with some working design notes in the comments.

One of the areas which we hope to address is better documentation for both users and contributors. @dsdinter recently added documentation on how to add a new metric:

README.MD

Please do feel free to make suggestions and provide feedback on the 'development' experience for creating new metrics. Its an area with a lot of rough edges at the moment!

Cheers

danp11 commented 3 years ago

Hi and thanks for your reply.

I'll create an issue later this week with a few comments on how I plan to implement VWAP and you can have a look at it and let me know if I'm on the right track. I'll follow the path outlined in the README

/Dan

danp11 commented 3 years ago

Hi again,

Is this a way forward?

Add VOLUME and PRICE to Indicators

Add VOLUME_WEIGHTED_AVERAGE_PRICE and MOVING_VOLUME_WEIGHTED_AVERAGE_PRICE to TSFSITechKeys

public abstract class VWAP implements Serializable

private BigDecimal price; private BigDecimal volume;

Also have enum public enum VWAPMethod { VOLUME_WEIGHTED_AVERAGE_PRICE, MOVING_VOLUME_WEIGHTED_AVERAGE_PRICE }

Thanks,

/Dan

rezarokni commented 3 years ago

Yes that is the general pattern, the are that could be interesting is the definition of VOLUME.

If VOLUME == DATA COUNT, in other words its a simple count of the number of elements of price seen, then this works as is.

If VOLUME needs to be derived from joining the price property with a quantity property from another TSKey then we need to add a joining function into the library , so that you can bring together values from different Type 1 computations ( the Combiner computations) into a single Type 2 computation.

I am looking at the 2nd option at the moment, there are a couple of ways of making this possible, either through a merge function or via a SideInput. The sideinput could be a nice generic option that allows metrics creators to simply bring in different keys into any Type 2 computation.

danp11 commented 3 years ago

Yes, the 2nd option would be a very good addition. Either via merge or sideinput. There are other metrics than depends on > 2 datapoints, so the more keys we can combine in a simple way, the better it is.

I'll wait to develop the VWAP for now and looking forward to add more metrics in the future.

Thanks, /Dan

rezarokni commented 3 years ago

Great, I have most of this ready in the following branch: https://github.com/GoogleCloudPlatform/dataflow-sample-applications/tree/TS-31-Add-Composite-TSAccum-Creation But it does touch on a lot of core files, so will add some more tests and a canonical example to show how to use it before merge to master.