Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong number of arguments in a call #291

Open
DimitriPapadopoulos opened this issue Nov 16, 2021 · 4 comments
Open

Wrong number of arguments in a call #291

DimitriPapadopoulos opened this issue Nov 16, 2021 · 4 comments

Comments

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Nov 16, 2021

Call to function scp_single with too few arguments; should be no fewer than 3.

Indeed the shape argument is missing:

for conf_threshold in conf_thresholds:
y_pred_above_confidence = _filter_y_pred(y_pred, conf_threshold)
ms.append(scp_single(y_true, y_pred_above_confidence))

See:
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

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Nov 16, 2021

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:

def scp_single(y_true, y_pred, shape, minipatch=None):

The shape argument of circle_maps() is not optional:

def circle_maps(y_true, y_pred, shape):

but it is documented to be optional, without a clear default value (None?):
shape : tuple of ints, optional
shape of image

The shape argument of project_circle() is optional and its default value is None:

def project_circle(circle, image=None, shape=None,
normalize=True, negative=False):

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?

map_true = np.zeros(shape)
map_pred = np.zeros(shape)

@DimitriPapadopoulos
Copy link
Contributor Author

So my questions are:

  1. Should shape be an optional argument for scp_single() and circle_maps()? If so, the default value would be None.
  2. Does 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).

@agramfort
Copy link
Contributor

@rth @kegl @albertcthomas do you have time to look?

thx @DimitriPapadopoulos

@kegl
Copy link
Contributor

kegl commented Dec 15, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants