TIFF wrapper compatibility broken due to all function names being lowercase.

[pieleric]

The latest git HEAD, since commit bdce54f, has broken compatibility with all our code.
All the methods of the TIFF class have been renamed to only be lowercase.

So simple code like this is broken:

    f = TIFF.open(fn, mode='r')
    if f.IsTiled():
        bits = f.GetField('BitsPerSample')
        sample_format = f.GetField('SampleFormat')
        typ = f.get_numpy_type(bits, sample_format)
        im = f.read_tiles(typ)
    else:
        im = f.read_image() # Fist image

Now, both .IsTiled() and .GetField() don't exist, as they've been renamed to .istiled() and getfield().
Note that the Python convention is to either have CamelCase, or lower_case names, however, it is not recommended to use lowercase without separation (because it makes names hard to read).

IMHO, in this case, as it's wrapper doing a simple mapping of C functions to Python methods, it makes sense to use the same convention as the C library, so that it's more obvious that they are just direct mapping. For the more complex methods, like read_image(), it's also fine to follow a different convention, as they are not a direct mapping of the C library, but additional helper code.

So, I'd suggest to revert the renaming of the methods. If needed, I can provide a patch.

[pearu]

Hi @jat255, @pieleric !

I agree that compatibility has been broken by the mentioned commit and we'll need to fix it. Actually, there are more than 20 methods that names were changed. All need a fix.

However, we need to take into account that now there exists users who use CamelCase and users who use lowercase in their applications.

While I would not normally use CamelCase for method names in Python programs, in this particular case, indeed the convention was taken from C library. Introducing lower_case would be third conventions that would make things even more confusing, IMHO.

I see the following possible solutions:

1. Use CamelCase and lowercase in parallel.
2. Use CamelCase and lowercase in parallel but give warning to a user when it uses lowercase: lowercase naming will be removed in future versions.
3. Use CamelCase and lowercase in parallel but give warning to a user when it uses CamelCase: CamelCase naming will be removed in future versions.

My preference order of solutions is 1, 2, 3.

What do you think?

Best regards,
Pearu

[geekofhearts]

It's relatively easy to use some python magic to just handle both versions for ever - I'd be in favour of that. (python magic - either assign via `Class.method_one = Class.MethodOne` or implement `__getattr__` and write a generic rule transforming the method names. Doing this second method via inheritance will make the code change very small indeed.

…

On 28 November 2016 at 13:37, Pearu Peterson ***@***.***> wrote: Hi @jat255 <https://github.com/jat255>, @pieleric <https://github.com/pieleric> ! I agree that compatibility has been broken by the mentioned commit and we'll need to fix it. Actually, there are more than 20 methods that names were changed. All need a fix. However, we need to take into account that now there exists users who use CamelCase and users who use lowercase in their applications. While I would not normally use CamelCase for method names in Python programs, in this particular case, indeed the convention was taken from C library. Introducing lower_case would be third conventions that would make things even more confusing, IMHO. I see the following possible solutions: 1. Use CamelCase and lowercase in parallel. 2. Use CamelCase and lowercase in parallel but give warning to a user when it uses lowercase: lowercase naming will be removed in future versions. 3. Use CamelCase and lowercase in parallel but give warning to a user when it uses CamelCase: CamelCase naming will be removed in future versions. My preference order of solutions is 1, 2, 3. What do you think? Best regards, Pearu — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAuh1cZrtEHVhBgM08YsxA0Rq-PBaz7Bks5rCtkWgaJpZM4K9ww2> .

[cmbruns-hhmi]

I agree with @pieleric that lowercasewithoutunderscores is a terrible naming convention. Perhaps having both CamelCase and lower_case_with_underscores would be OK, as long as someone is willing to maintain this convention.

It looks to me like commit bdce54f was an overeager underthought commit. It combines so many changes together in one giant blob. The commit message claims to be supporting PEP8. But, for example, the systematic removal of spaces around the assignment operator is the exact opposite of what PEP8 demands. (The only place such spaces should be removed is when setting named argument values in function calls.)

[pearu]

When using Python magic, let us be explicit:

def CamelCase(...):
  ...
camelcase = camel_case = CamelCase

[jat255]

@cmbruns-hhmi I agree completely. I needed this library to work for my compatibility and it hasn't been updated in years, so I just submitted what I needed. Feel free to change it however is needed. No offense taken...

[pieleric]

If it was just up to me, I'd just drop the lowercase method names, as they've never been there in any stable release (as far as I understand).
However, as it's not in any of the options @pearu is proposing, then I'd just tend toward solution 1 and keep the code simple. In particular, let's not bring in a third name for the methods (ie, lower_case), that really not usual for Python libraries to offer every possible naming convention for all the methods.

[pearu]

Thanks, @pieleric!