search

world-federation-of-advertisers / cardinality_estimation_evaluation_framework

Evaluation framework and methods for estimating cardinalities of groups of sets
Apache License 2.0
22 stars 9 forks source link

Unused and invalid `app.run` #126

Closed riemanli closed 3 years ago

riemanli commented 3 years ago

https://github.com/world-federation-of-advertisers/cardinality_estimation_evaluation_framework/blob/732117efd307230156376e88f4d2ab3bb6b5df97/src/common/noisers.py#L229-L235

Hi. Do we need the code shown in the snippet above? main doesn't do anything but check number of command-line arguments. Plus, absl.app is not imported in this module, which causes problems.

matthewclegg commented 3 years ago

Seems unnecessary to me.

On Thu, Oct 14, 2021, 1:54 PM Rieman @.***> wrote:

https://github.com/world-federation-of-advertisers/cardinality_estimation_evaluation_framework/blob/732117efd307230156376e88f4d2ab3bb6b5df97/src/common/noisers.py#L229-L235

Hi. Do we need the code shown in the snippet above? main doesn't do anything but check number of command-line arguments. Plus, absl.app is not imported in this module, which causes problems.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/world-federation-of-advertisers/cardinality_estimation_evaluation_framework/issues/126, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR7TBWB6C6DZNEN3ZK2OG3UG47PXANCNFSM5GARFDIA .

riemanli commented 3 years ago

Submit PR #127 to remove the redundant code.

jiayu-google commented 3 years ago

Agreed, unnecessary to me.

riemanli commented 3 years ago

https://github.com/world-federation-of-advertisers/cardinality_estimation_evaluation_framework/blob/732117efd307230156376e88f4d2ab3bb6b5df97/src/estimators/estimator_noisers.py#L124-L130

Found one more file has the same issue. Removed it in the same PR #127.

riemanli commented 3 years ago

Fixed in PR #127.