Skip to content

Custom _autocast function #15

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Custom _autocast function #15

wants to merge 4 commits into from

Conversation

rjarry
Copy link
Contributor

@rjarry rjarry commented Sep 3, 2012

Hey ntt :)

Do you remember talking about an "official" way of having a custom _autocast function without monkey patching your module?

I've made something in that direction. I'm pretty sure I didn't break anything and that it would be backward compatible. Could you integrate it?

Thanks o/
diab

@ntt
Copy link
Owner

ntt commented Sep 4, 2012

apply() was deprecated ages ago.
and wrapping it in a method? ohgodwhy.

This introduces at least 2 additional levels of function calls per item cast, which isn't something you want when parsing huge items like alliancelist and transactions.

I think I'll do it my way ;-) There's nothing wrong with monkeypatching btw. it's just less elegant.

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

Yeah I didn't want to use apply but self.autocast was passing self as first parameter. I don't know how to do it differently.

About monkeypatching, that's what i tried on my project but it seems not to work in random cases. Maybe you could help?

@ntt
Copy link
Owner

ntt commented Sep 4, 2012

what do you mean not work in random cases. as long as you patch immediately after the first "import eveapi" statement, it should work fine.

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

Here is the monkeypatch that seems not to work in some random cases:

http://eve-corp-management.org/projects/ecm/repository/entry/ecm/lib/eveapi_patch.py

It is initialized/applied like that :

http://eve-corp-management.org/projects/ecm/repository/entry/ecm/apps/common/api.py

Then we use our method to create a EVEAPIConnection object:

http://eve-corp-management.org/projects/ecm/repository/entry/ecm/apps/corp/tasks/standings.py

The problem is that some (random) users face this error:

Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/ecm-2.1.0.dev-py2.6.egg/ecm/apps/corp/tasks/standings.py", line 42, in update
    currentTime = timezone.make_aware(corpApi._meta.currentTime, timezone.utc)
  File "/usr/local/lib/python2.6/dist-packages/Django-1.4.1-py2.6.egg/django/utils/timezone.py", line 269, in make_aware
    return timezone.localize(value, is_dst=None)
  File "/usr/local/lib/python2.6/dist-packages/pytz-2012d-py2.6.egg/pytz/__init__.py", line 230, in localize
    if dt.tzinfo is not None:
AttributeError: 'int' object has no attribute 'tzinfo'

Which means that the default _autocast function is used instead of ours. And frankly I don't understand why...

I know this isn't stackoverflow but maybe you can help :P

@ntt
Copy link
Owner

ntt commented Sep 4, 2012

Hm, Maybe you should log everything going through autocast. Could be that it's not recognizing a date properly.

Or is it working fine if you replace the autocast function in eveapi itself?

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

It works fine most of the time. And the fact that we have an int says that it goes through the default _autocast (else we would have a plain string).

We added some logging and here's what we got:

2012-09-04 13:54:10,982 [DEBUG] ecm.eveapi - OFFICIAL eveapi imported
2012-09-04 13:54:10,986 [DEBUG] ecm.lib.eveapi_patch - ecm.lib.eveapi_patch imported
2012-09-04 13:54:10,986 [DEBUG] ecm.lib.eveapi_patch - eveapi patched
2012-09-04 13:54:20,317 [INFO ] ecm.apps.corp.tasks.corp - fetching /corp/CorporationSheet.xml.aspx...
2012-09-04 13:54:20,379 [ERROR] ecm.apps.scheduler.models -
Traceback (most recent call last):
  File "/var/www/eve-corp-management/ecm/apps/scheduler/models.py", line 120, in run
    func(**args)
  File "/var/www/eve-corp-management/ecm/apps/corp/tasks/corp.py", line 51, in update
    currentTime = timezone.make_aware(corpApi._meta.currentTime, timezone.utc)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.4.1-py2.7.egg/django/utils/timezone.py", line 269, in make_aware
    return timezone.localize(value, is_dst=None)
  File "/usr/local/lib/python2.7/dist-packages/pytz-2012d-py2.7.egg/pytz/__init__.py", line 230, in localize
    if dt.tzinfo is not None:
AttributeError: 'int' object has no attribute 'tzinfo'

edit: when we directly modify the default _autocast function, it works fine :)

@ntt
Copy link
Owner

ntt commented Sep 4, 2012

bizarre :)

@ntt
Copy link
Owner

ntt commented Sep 4, 2012

try raising an exception in the original autocast :)

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

Forget it, this error is too much random and "bizarre" as you say ^^

I'll wait for the official way of using a custom _autocast function to be integrated. It will be simpler :D

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

Is it better like this? :)

@ntt
Copy link
Owner

ntt commented Sep 4, 2012

you know you can just self.autocast(bla) right?

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

Well, normally self.autocast(bla) will call the autocast function (not method) with self as a first parameter.

It means that the default _autocast function will not work as it is. If you want it to work with self.autocast(bla) you need to pass a function that accepts 3 parameters and only consider the last 2 ones.

Something like this:

def _autocast(self, key, value):
    # attempts to cast an XML string to the most probable type.
    try:
        if value.strip("-").isdigit():
            return int(value)
    except ValueError:
        pass

    try:
        return float(value)
    except ValueError:
        pass

    if len(value) == 19 and value[10] == ' ':
        # it could be a date string
        try:
            return max(0, int(timegm(strptime(value, "%Y-%m-%d %H:%M:%S"))))
        except OverflowError:
            pass
        except ValueError:
            pass

    # couldn't cast. return string unchanged.
    return value

(or I didn't understand a thing about the python object system)

@rjarry
Copy link
Contributor Author

rjarry commented Sep 4, 2012

Ok forget what I said

I'm not replacing _Parser.autocast but self.autocast in the constructor of _Parser.

I wasn't aware that it behaved differently in that case.

@ntt
Copy link
Owner

ntt commented Sep 5, 2012

Yep, any functions defined in a class accessed through an instance of it will be bound (ie. get reference to class instance added as first parameter).
They can even be added after class creation, and any existing instances will magically have the new method then too.
Just assigning a function to an instance variable doesn't do anything magic like that.

@rjarry
Copy link
Contributor Author

rjarry commented Sep 6, 2012

Hello there :)

Do you plan to integrate this feature soon(tm) ? We're kind of waiting for it to make a new release :)

cheers o/

@ntt
Copy link
Owner

ntt commented Sep 7, 2012

ohgod, pressure :P

@rjarry
Copy link
Contributor Author

rjarry commented Sep 20, 2012

Pressure

:D

@tyler274
Copy link

any update on this...

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 this pull request may close these issues.

3 participants