search

tnc-br / ddf-isoscapes

3 stars 0 forks source link

add stamp for canon p_value_threshold #187

Closed VimWizardVersace closed 12 months ago

review-notebook-app[bot] commented 12 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

VimWizardVersace commented 12 months ago

as discussed in chat, lets make the min radius 5km and change the key name for 3000km to be "MAX" those should be minor changes

I set the min_radius to 5 (min_fraud_radius) but kept the steps in increments of 100 hence this extra logic. (This was a request by Dr. Martinelli)

Also, there a bug in how the precision, recall, and p_value thresholds were copied so the diff might be slightly larger than expected.

VimWizardVersace commented 12 months ago

In plot_isoscape_precision_recall, are we using the new radius argument in some way?

yes, in plt.title

Previously, it grabbed the radius variable from the global scope. This was bad as "radius" was mentioned and set in a bunch of places.

VimWizardVersace commented 12 months ago

In plot_isoscape_precision_recall, are we using the new radius argument in some way?

yes, in plt.title

Previously, it grabbed the title variable from the global scope. This was bad as "radius" was mentioned and set in a bunch of places.

I dont think you need to rename the stamp method anymore as it does it all. Looks good.

Reverted