geopython / pygeofilter

pygeofilter is a pure Python parser implementation of OGC filtering standards
MIT License
82 stars 31 forks source link

colon in attribute name #43

Closed srepmub closed 2 years ago

srepmub commented 2 years ago

hi,

I am trying to filter on an attribute with a colon in its name, in my case 's5p:orbit'. after conversion to django filters, this name is unchanged, so django cannot find the matching field. so I now manually change the AST so colons become " ".. of course while typing this, I see that you can pass a field mapping to tofilter.. so now I'm wondering if it would make sense to have a mode where ':' is automatically converted to " _", for cases where the django model matches this?

constantinius commented 2 years ago

@srepmub There is a field_mapping option specifically for this reason. Could you try that?

srepmub commented 2 years ago

thank you. yes I'm sure this will work, but I guess my suggestion is to automate this for when the ':' can be resolved automatically to a related django model. in other words, for above example, if 's5p' is a field in the current django model that relates to another model.

constantinius commented 2 years ago

@srepmub I see. I think this can be done with the field_mapping as well, but not using a dict, but a custom mapping class. e.g:

class FieldMapper:
    def __getitem__(self, key):
        return key.replace(":", "__")

This would translate "s5p:orbit" to "s5p__orbit". Is this more what you want?

constantinius commented 2 years ago

Ah, you actually have to override the get method as well:

class FieldMapper:

    def __getitem__(self, key):
        return key.replace(":", "__")

    def get(self, key, default=None):
        return key.replace(":", "__")
srepmub commented 2 years ago

ah yes that works, thanks :) still I feel like this could also be handled automatically, by looking at the django model and resolving against it. but feel free to close this issue if that doesn't seem useful to you or worth the effort.

constantinius commented 2 years ago

I will close this issue now, as there is a way to do it, but I'd still like to have the discussion open.

How would your proposed solution look like?

srepmub commented 2 years ago

I would have to check, but I can imagine that ':' is not even legal in a django field name..? so you could perhaps by default change ':' into '__', so unless there is an actual field_mapping provided that overrides this?