pygame.image docs missing functions and load_basic ignore namehint argument

[MightyJosip]

pygame/src_c/image.c

Line 62 in ea4a028

image_load_basic(PyObject *self, PyObject *arg)

This can be seen in the function above. name variable gets parsed, but never used. Also there are missing functions in docs: ('load_basic', 'load_extended', 'save_extended')

Related Docs: https://www.pygame.org/docs/ref/image.html

To Do to Close

• Doc string for function 'load_basic'.
• Doc string for function 'load_extended'.
• Doc string for function 'save_extended'
• Regular .rst docs for these functions.
• Investigate unused namehint argument for load_basic.

[ankith26]

I guess I have answer to the last two questions. In the case of pygame.image.load_basic(), the namehint argument serves no purpose because the function is for loading BMP images anyways, it does not need the help of namehint argument to determine what kind of image it needs to load.

But it is still required nonetheless because when SDL-image module is not used while building pygame, pygame.image.load() is going to fall back to using pygame.image.load_basic(), and pygame.image.load() takes the optional name-hint argument, so even load_basic() must take the optional argument.

[ankith26]

And yes, I want to be documenting all the other functions (and maybe some minor fixes to pygame.image docs)

Edit: I feel there is no need to document load_basic(), load_extended() and save_extended() as they are more of internal methods, and the load() function and save() function directly call these. Users are using load() and save() anyways. But maybe we could document pygame.image.load_basic() as an exclusive way to load only bmp images.

[siggisv]

In my opinion, @ankith26 is right that there is no need to document load_basic(), load_extended() and save_extended().

Furthermore, I feel that not only should they not be documented, the should be renamed to _load_basic(), _load_extended() and _save_extended() to make it clearer that they are helper functions.

Regarding the unused name-hint argument, it should be easy to change load_basic() to check it and return error if it is not ending with "BMP" (or "bmp").

[MyreMylar]

I agree that load_basic(), load_extended() and save_extended() should be renamed with the preceding underscore so they don't appear in the public interface...

However, a quick GitHub search reveals that there are already lots of people using them, despite them being undocumented. See:

https://github.com/search?q=pygame.image.load_basic%28&type=code
https://github.com/search?q=pygame.image.load_extended%28&type=code

Which is the perpetual curse of working with pygame's age and large userbase.

[siggisv]

Well, that is a problem.

Maybe we could:

• Start with writing in the documentation that those are "internal helper functions that might be renamed or deleted at any time and therefore should not be used directly. Use the load() and save() functions instead."
• Then (later?) rename those functions while at the same time create new functions with the old names whose only purpose is to give a deprecation warning before passing the call to the original function (e.g. the pseudocode for the "new" load_basic() function would be: warning(deprecated); _load_basic(args);)

[ankith26]

What I feel is that renaming or removing these helper functions is going to be a problem for all those projects using these functions.

My take on this problem is to keep these functions as-is, document these three functions, but put a side note, recommending people to use load() and save() instead. If documenting the functions is the way to go, then I’m going to do it as a part of #2130

[MyreMylar]

I personally have no objection to deprecating the public versions of these functions. I would leave them working but with a deprecation warning for the 2.0.0 release and then clear them out in 2.1/2.2.

[ankith26]

Code changes may follow later, but I would go ahead and put in the docs, the message to discourage the use of these functions (as a part of PR #2130)

[ankith26]

Is this issue sorted, can it be closed?

[MyreMylar]

Deprecating these from the public interface is likely not that urgent so we can probably roll that work up in the general debate over deprecation warnings. However, the results for these are a bit wrong/useless:

help(pygame.image.load_extended)
help(pygame.image.save_extended)
help(pygame.image.load_basic)

It's be nice if their doc strings explained what the function did and why you shouldn't use them. I've updated the ToDo list.

[ankith26]

I will put up a PR for this. (image.c will need a bit of reworking)

[ankith26]

I’m not currently working on this, if anyone else is interested, feel free to take this up.

Edit: actually I’m kind of working on this issue, it’s just that if I can’t figure out how to do it (I’m a bit new to C programming), then I will have to give up

[ankith26]

#2270 fixes this issue

[ankith26]

illume noted, on my PR

I don't think these functions are deprecated, or will be. They're still useful, and used fairly widely.

Can you please add these docs into the .rst file, and have them come in via the doc headers (generated with python setup.py docs)?

And I kinda agree too, atleast for the function load_basic(). This function looks like it is used directly in many places.