mwvgroup / pittgoogle-user-demos

GNU General Public License v3.0
2 stars 0 forks source link

Update supernova classifier module version from `v0.6` to `v0.7` #22

Closed hernandezc1 closed 1 year ago

hernandezc1 commented 1 year ago

Since the start of the original ELAsTiCC challenge, the supernova classifier module has undergone a few changes. These changes were introduced without declaring an updated version (currently v0.6). For the ELAsTiCC2 challenge, we are formalizing the module's version as v0.7.

For the ELAsTiCC2 challenge, we have:

Future updates to the classifier module will result in updated version declarations.

Should this PR should be merged after merging PR #20?

troyraen commented 1 year ago

I suppose we should add this "brokerVersion" to the BigQuery table as well since otherwise we'll have no permanent record of which alerts were processed using which version. Let's stick with the field name "brokerVersion" to match the outgoing Avro schema, but clarify in the field description that this is the version of our classifier module.

Should this PR should be merged after merging PR https://github.com/mwvgroup/pittgoogle-user/pull/20?

No, this PR should go through ASAP to track the changes that have already been made.

hernandezc1 commented 1 year ago

I added the brokerVersion to the BigQuery table and tested it. You can review the BigQuery table here

hernandezc1 commented 1 year ago

I've updated main.py so that brokerVersion is only defined once

I strongly recommend defining variables only once. Here you're defining the same brokerVersion in two different places. This makes it easy to introduce errors in the future because it's easy to forget (or for others to not even realize) that the version has to be updated in two different places. Our handling of this variable will improve a bit after #20 but I still recommend changing it here to avoid the problem altogether.