Open DimitriPapadopoulos opened 2 years ago
I see a more general issue with the shape
argument in the call tree:
scp_single (y_true, y_pred, shape, minipatch=None)
└── circle_maps (y_true, y_pred, shape)
├── numpy.zeros (shape)
└── project_circle (circle, image=None, shape=None, normalize=True, negative=False)
The shape
argument of scp_single()
is not optional:
https://github.com/paris-saclay-cds/ramp-workflow/blob/7d075219d42cf5c8cca62c6c9804dba8c04907cc/rampwf/score_types/detection/scp.py#L52
The shape
argument of circle_maps()
is not optional:
https://github.com/paris-saclay-cds/ramp-workflow/blob/7d075219d42cf5c8cca62c6c9804dba8c04907cc/rampwf/score_types/detection/scp.py#L139
but it is documented to be optional, without a clear default value (None
?):
https://github.com/paris-saclay-cds/ramp-workflow/blob/7d075219d42cf5c8cca62c6c9804dba8c04907cc/rampwf/score_types/detection/scp.py#L153-L154
The shape
argument of project_circle()
is optional and its default value is None
:
https://github.com/paris-saclay-cds/ramp-workflow/blob/7d075219d42cf5c8cca62c6c9804dba8c04907cc/rampwf/score_types/detection/scp.py#L93-L94
Note that numpy.zeros(None)
is valid and returns array(0.0)
. Is that what is expected when the shape
argument of circle_maps()
is None
?
https://github.com/paris-saclay-cds/ramp-workflow/blob/7d075219d42cf5c8cca62c6c9804dba8c04907cc/rampwf/score_types/detection/scp.py#L161-L162
So my questions are:
shape
be an optional argument for scp_single()
and circle_maps()
? If so, the default value would be None
.project_circle()
do the right thing when shape is None
? I haven't looked into the function in enough detail to have an opinion (I could do that but I thought I would ask first).@rth @kegl @albertcthomas do you have time to look?
thx @DimitriPapadopoulos
Hm, this was done mostly by @jorisvandenbossche if I'm not mistaken. Are you guys @agramfort @DimitriPapadopoulos are planning to use this in an upcoming challenge? In this case I'd review the whole detection pipeline. I have no objection on PRs that fix this or anything else in detection (on the contrary, pls go ahead, thanks a lot!), it's well encapsulated within the RAMP detection framework and should not affect anything else.
Call to function
scp_single
with too few arguments; should be no fewer than 3.Indeed the
shape
argument is missing: https://github.com/paris-saclay-cds/ramp-workflow/blob/7d075219d42cf5c8cca62c6c9804dba8c04907cc/rampwf/score_types/detection/util.py#L214-L216See: https://lgtm.com/projects/g/paris-saclay-cds/ramp-workflow/snapshot/97b80f6dbfaf877cc5d8f807d90fd2986978019e/files/rampwf/score_types/detection/util.py?sort=name&dir=ASC&mode=heatmap#x31a3fdb0293f3757:1