Display bug with User Category

[Wiggo]

There's a strange bug in the book details with my alias column.
The name is cutted off, and it happens with this name only.

Edit: additional info

def evaluate(self, formatter, kwargs, mi, locals, val, col_name, user_cat_prefix):
    new_val = ''
    if hasattr(mi, '_proxy_metadata'):
        all_cats = mi._proxy_metadata.user_categories
        cats = {k:v for k,v in all_cats.items() if k.startswith(user_cat_prefix)}
        SEP = mi.metadata_for_field(col_name)['is_multiple'].get('list_to_ui', '')
        new_val = set()
        if SEP:
            val_ = val.split(SEP)
        else:
            val_ = [val]  
        for user_cat, v in cats.items():
            repl = user_cat.lstrip(user_cat_prefix)
            for user_cat_item, src_cat in v:
                if src_cat == col_name:
                    for item in val_[:]:
                        if item == user_cat_item:
                            new_val.add(repl)
                            val_.remove(item)
                        else:
                            new_val.add(item)
        if new_val:
            return ', '.join(list(new_val))
    return val

[dunhill]

Where do you put the code, I find the idea interesting

[chaley]

Originally Posted by Wiggo

There's a strange bug in the book details with my alias column.
The name is cutted off, and it happens with this name only.

It doesn't fail for me.

Is the 'P' in 'Petra' a non-Latin unicode character? I can see something like this happening if it is a unicode composed character.

FWIW: there is some strange (to me) code in your function:

def evaluate(self, formatter, kwargs, mi, locals, val, col_name, user_cat_prefix):
    new_val = ''
    if hasattr(mi, '_proxy_metadata'):
        # Below should be mi.user_categories
        all_cats = mi._proxy_metadata.user_categories
        cats = {k:v for k,v in all_cats.items() if k.startswith(user_cat_prefix)}
        SEP = mi.metadata_for_field(col_name)['is_multiple'].get('list_to_ui', '')
        new_val = set()
        if SEP:
            val_ = val.split(SEP)
        else:
            val_ = [val]  
        for user_cat, v in cats.items():
            repl = user_cat.lstrip(user_cat_prefix)
            for user_cat_item, src_cat in v:
                if src_cat == col_name:
                    # Why does the next line have the slice?
                    # I think it should be 
                    # for item in val_:
                    for item in val_[:]:
                        if item == user_cat_item:
                            new_val.add(repl)
                            # Why are you removing the item from the list?
                            # Sometimes that will mess up the for loop. Or
                            # at least it could in python 2.
                            val_.remove(item)
                        else:
                            new_val.add(item)
        if new_val:
            return ', '.join(list(new_val))
    return val

[dunhill]

@Chaley where does the code go in the Pseudo custom column???

[capink]

Originally Posted by chaley

It doesn't fail for me.

Is the 'P' in 'Petra' a non-Latin unicode character? I can see something like this happening if it is a unicode composed character.

FWIW: there is some strange (to me) code in your function:

def evaluate(self, formatter, kwargs, mi, locals, val, col_name, user_cat_prefix):
    new_val = ''
    if hasattr(mi, '_proxy_metadata'):
        # Below should be mi.user_categories
        all_cats = mi._proxy_metadata.user_categories
        cats = {k:v for k,v in all_cats.items() if k.startswith(user_cat_prefix)}
        SEP = mi.metadata_for_field(col_name)['is_multiple'].get('list_to_ui', '')
        new_val = set()
        if SEP:
            val_ = val.split(SEP)
        else:
            val_ = [val]  
        for user_cat, v in cats.items():
            repl = user_cat.lstrip(user_cat_prefix)
            for user_cat_item, src_cat in v:
                if src_cat == col_name:
                    # Why does the next line have the slice?
                    # I think it should be 
                    # for item in val_:
                    for item in val_[:]:
                        if item == user_cat_item:
                            new_val.add(repl)
                            # Why are you removing the item from the list?
                            # Sometimes that will mess up the for loop. Or
                            # at least it could in python 2.
                            val_.remove(item)
                        else:
                            new_val.add(item)
        if new_val:
            return ', '.join(list(new_val))
    return val

I wrote this a long time ago, so I don't remember the details. The slice was intentional to create a new list (a substitute for copy.copy()), because I needed to preserve the original one (I don't remember the details now).

Edit: Looking at the code now, I guess the slicing might have been an attempt to improve performance of the template function, by not having to iterate over items that have been already processed.

The mi._proxy_metadata.user_categories was the way I saw it done in one of the builtin template functions. But since you recommend mi.user_categories(), I will be using that instead.

[capink]

Originally Posted by dunhill

@Chaley where does the code go in the Pseudo custom column???

You can find more details for how to use this function example no. 4 in this link.

[chaley]

Originally Posted by capink

I wrote this a long time ago, so I don't remember the details. The slice was intentional to create a new list (a substitute for copy.copy()), because I needed to preserve the original one (I don't remember the details now).

Edit: Looking at the code now, I guess the slicing might have been an attempt to improve performance of the template function, by not having to iterate over items that have been already processed.

That makes sense, given the .remove(). You certainly don't want to change the list maintained by the db layer! Have you checked that you can safely remove an item in the list in the current for iterator? I don't know ...

The mi._proxy_metadata.user_categories was the way I saw it done in one of the builtin template functions. But since you recommend mi.user_categories(), I will be using that instead.

Yes, I see now that I did that. I don't remember why. Regardless, it isn't needed. If user_categories is referenced (it is an attribute, not a function) then the "right thing" will happen no matter if the base class is a proxy_metadata instance or a 'real' mi instance. In most cases it will be a proxy_metadata instance. I will (eventually) change the reference in formatter_functions.2,315: def user_categories_for_books()

[Wiggo]

Originally Posted by chaley

It doesn't fail for me.

Is the 'P' in 'Petra' a non-Latin unicode character? I can see something like this happening if it is a unicode composed character.

I typed the "P" normally with my keyboard. When I type "Petra", "tra" appears and when I type "Pitra", "itra" appears.
It's the "P" only. I tried different entries changing e.g. "Robert" to "Pobert" and the same thing happened


My problem is that I do not know where to look for the error.

@capink: Will you update your template?

[chaley]

The problem happens because the template function uses lstrip() to remove the prefix. That function doesn't look for a string but instead strips out each character (case sensitive). Your user category is Pseudo so if an author begins with any of those letters (note that P is one of them) then it will be stripped. If an author name started with d then the same thing will happen.

This fixes it by changing lstrip() to a slice, removing N characters from the beginning where N is the length of the outer user category name passed to the function (user_cat_prefix). As a side benefit it will be a bit faster.

def evaluate(self, formatter, kwargs, mi, locals, val, col_name, user_cat_prefix):
    new_val = ''
    if hasattr(mi, '_proxy_metadata'):
        all_cats = mi.user_categories
        cats = {k:v for k,v in all_cats.items() if k.startswith(user_cat_prefix)}
        SEP = mi.metadata_for_field(col_name)['is_multiple'].get('list_to_ui', '')
        new_val = set()
        if SEP:
            val_ = val.split(SEP)
        else:
            val_ = [val]  
        prefix_length = len(user_cat_prefix)
        for user_cat, v in cats.items():
            repl = user_cat[prefix_length:]
            for user_cat_item, src_cat in v:
                if src_cat == col_name:
                    for item in val_:
                        if item == user_cat_item:
                            new_val.add(repl)
                            val_.remove(item)
                        else:
                            new_val.add(item)
        if new_val:
            return ', '.join(list(new_val))
    return val

[capink]

Originally Posted by chaley

This fixes it by changing lstrip() to a slice, removing N characters from the beginning where N is the length of the outer user category name passed to the function (user_cat_prefix). As a side benefit it will be a bit faster.

I overlooked this, Thanks for the fix. I am thinking about going with re.sub() in case the user enters a wrong prefix. It will be slower though.

[chaley]

Originally Posted by capink

I overlooked this, Thanks for the fix.

I looked at that code at least 20 times before I saw it.

I am thinking about going with re.sub() in case the user enters a wrong prefix. It will be slower though.

I don't understand what re.sub() would fix. The dict comprehension on line 5 ensures that the key starts with the user_cat_prefix so removing len(user_cat_prefix) characters from the key (user_cat) can't do the wrong thing. What am I missing?

[capink]

Originally Posted by chaley

I looked at that code at least 20 times before I saw it.
I don't understand what re.sub() would fix. The dict comprehension on line 5 ensures that the key starts with the user_cat_prefix so removing len(user_cat_prefix) characters from the key (user_cat) can't do the wrong thing. What am I missing?

It will not change much. It only fails better in case the user enters a wrong value for user_cat_prefix (e.g missing one or more letters), it will return the full path of the user category, which makes it more obvious what the user did wrong.

[chaley]

Originally Posted by capink

It will not change much. It only fails better in case the user enters a wrong value for user_cat_prefix (e.g missing one or more letters), it will return the full path of the user category, which makes it more obvious what the user did wrong.

Ahh, I see. That makes sense. However, don't you need to change/get rid of the dict comprehension on line 5? It ensures that only user categories with the prefix used are put into the "cats" dict, making it impossible for the re.sub() to fail.

[Wiggo]

@chaley

I don't even know what to say - thank you so much!

I think it's really great how you spend your time and help noobs like me out of trouble.

[chaley]

Originally Posted by capink

The mi._proxy_metadata.user_categories was the way I saw it done in one of the builtin template functions. But since you recommend mi.user_categories(), I will be using that instead.

And you were right. There are cases where mi.user_categories gives the wrong answer, while mi._proxy_metadata.user_categories always works. The problems can arise in device and plugboard templates.

Sorry about the noise in the channel.

Originally Posted by Wiggo

@chaley

I don't even know what to say - thank you so much!

I think it's really great how you spend your time and help noobs like me out of trouble.

You are welcome. And it was partly for me. I needed to be sure that there wasn't a strange bug somewhere in template processing.