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

add ipython pretty printing for AttribDict + Stats, finally #1790

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@megies
Member

megies commented May 18, 2017

Ever found yourself typing something like the following in IPython to get readable output from an AttribDict?

[In 10]: st[0].stats.sac.__dict__

Me too, and I'm kind of sick of AttribDicts not pretty printing in IPython. So here goes.

From:

In [1]: from obspy.core.util import AttribDict

In [2]: xx = AttribDict({u'sensitivity_frequency': 0.02, u'name': 'RJOB.2007.351.HZ', u'sensitivity': 2516800000.0, u'normalization_frequency': 1.0, u'sensor_manufacturer': 'St
   ...: reckeisen', u'sensitivity_unit': 'M/S', u'normalization_factor': 60077000.0, u'zeros': [0j, 0j], u'gain': 60077000.0, u'poles': [(-0.037004+0.037016j), (-0.037004-0.037
   ...: 016j), (-251.33+0j), (-131.04-467.29j), (-131.04+467.29j)], u'sensor_model': 'STS-2', u'response_type': 'A'})

In [3]: xx
Out[3]: AttribDict({u'normalization_factor': 60077000.0, u'name': 'RJOB.2007.351.HZ', u'sensitivity': 2516800000.0, u'normalization_frequency': 1.0, u'sensor_manufacturer': 'St
reckeisen', u'sensitivity_unit': 'M/S', u'sensitivity_frequency': 0.02, u'zeros': [0j, 0j], u'gain': 60077000.0, u'poles': [(-0.037004+0.037016j), (-0.037004-0.037016j), (-251.
33+0j), (-131.04-467.29j), (-131.04+467.29j)], u'sensor_model': 'STS-2', u'response_type': 'A'})

In [4]: st = read('/path/to/test.sac')

In [5]: st[0].stats
Out[5]: 
         network: 
         station: STA
        location: 
         channel: Q
       starttime: 1978-07-18T08:00:10.000000Z
         endtime: 1978-07-18T08:01:49.000000Z
   sampling_rate: 1.0
           delta: 1.0
            npts: 100
           calib: 1.0
         _format: SAC
             sac: AttribDict({u'nzyear': 1978, u'nzjday': 199, u'nzhour': 8, u'lcalda': 1, u'iftype': 1, u'nvhdr': 6, u'unused23': 0, u'depmin': -1.0, u'kcmpnm': u'Q       ', u
'nzsec': 0, u'kevnm': u'FUNCGEN: SINE   ', u'depmen': 8.3446501e-08, u'depmax': 1.0, u'lovrok': 1, u'delta': 1.0, u'nzmsec': 0, u'lpspol': 0, u'b': 10.0, u'e': 109.0, u'leven':
 1, u'kstnm': u'STA     ', u'nzmin': 0, u'npts': 100})

screenshot from 2017-05-18 12 57 55


To:

In [1]: from obspy.core.util import AttribDict

In [2]: xx = AttribDict({u'sensitivity_frequency': 0.02, u'name': 'RJOB.2007.351.HZ', u'sensitivity': 2516800000.0, u'normalization_frequency': 1.0, u'sensor_manufacturer': 'St
   ...: reckeisen', u'sensitivity_unit': 'M/S', u'normalization_factor': 60077000.0, u'zeros': [0j, 0j], u'gain': 60077000.0, u'poles': [(-0.037004+0.037016j), (-0.037004-0.037
   ...: 016j), (-251.33+0j), (-131.04-467.29j), (-131.04+467.29j)], u'sensor_model': 'STS-2', u'response_type': 'A'})

In [3]: xx
Out[3]: 
AttribDict({u'gain': 60077000.0,
            u'name': 'RJOB.2007.351.HZ',
            u'normalization_factor': 60077000.0,
            u'normalization_frequency': 1.0,
            u'poles': [(-0.037004+0.037016j),
             (-0.037004-0.037016j),
             (-251.33+0j),
             (-131.04-467.29j),
             (-131.04+467.29j)],
            u'response_type': 'A',
            u'sensitivity': 2516800000.0,
            u'sensitivity_frequency': 0.02,
            u'sensitivity_unit': 'M/S',
            u'sensor_manufacturer': 'Streckeisen',
            u'sensor_model': 'STS-2',
            u'zeros': [0j, 0j]})

In [4]: st = read('/path/to/test.sac')

In [5]: st[0].stats
Out[5]: 15

        network: u''
        station: u'STA'
       location: u''
        channel: u'Q'
      starttime: 1978-07-18T08:00:10.000000Z
        endtime: 1978-07-18T08:01:49.000000Z
  sampling_rate: 1.0
          delta: 1.0
           npts: 100
          calib: 1.0
        _format: 'SAC'
            sac: AttribDict({u'b': 10.0,
                             u'delta': 1.0,
                             u'depmax': 1.0,
                             u'depmen': 8.3446501e-08,
                             u'depmin': -1.0,
                             u'e': 109.0,
                             u'iftype': 1,
                             u'kcmpnm': u'Q       ',
                             u'kevnm': u'FUNCGEN: SINE   ',
                             u'kstnm': u'STA     ',
                             u'lcalda': 1,
                             u'leven': 1,
                             u'lovrok': 1,
                             u'lpspol': 0,
                             u'npts': 100,
                             u'nvhdr': 6,
                             u'nzhour': 8,
                             u'nzjday': 199,
                             u'nzmin': 0,
                             u'nzmsec': 0,
                             u'nzsec': 0,
                             u'nzyear': 1978,
                             u'unused23': 0})

screenshot from 2017-05-18 12 58 10
screenshot from 2017-05-18 12 58 21

@megies megies added this to the 1.1.0 milestone May 18, 2017

@megies megies requested review from QuLogic and krischer May 18, 2017

@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer May 18, 2017

Member

Wohoo :) While at it: can you also drop the u prefix for unicode strings in Python 2 in the pretty representation?

Member

krischer commented May 18, 2017

Wohoo :) While at it: can you also drop the u prefix for unicode strings in Python 2 in the pretty representation?

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 18, 2017

Member

I'm also proposing to add meaningful error messages when equality tests of Stats/AttribDict fail.

The most useful and cleanest way to implement this is to directly plug it into TestCase.assertEqual via TestCase.addTypeEqualityFunc. To make this work in both obspy-runtest and py.test scenarios, I don't see a way around subclassing unittest.TestCase but I don't see negative side effects, currently.

Before:
screenshot from 2017-05-18 13 59 25

After:
screenshot from 2017-05-18 13 59 36

Motivation for this change: Incredibly annoying failure reports like these:

http://tests.obspy.org/84079/#2

Member

megies commented May 18, 2017

I'm also proposing to add meaningful error messages when equality tests of Stats/AttribDict fail.

The most useful and cleanest way to implement this is to directly plug it into TestCase.assertEqual via TestCase.addTypeEqualityFunc. To make this work in both obspy-runtest and py.test scenarios, I don't see a way around subclassing unittest.TestCase but I don't see negative side effects, currently.

Before:
screenshot from 2017-05-18 13 59 25

After:
screenshot from 2017-05-18 13 59 36

Motivation for this change: Incredibly annoying failure reports like these:

http://tests.obspy.org/84079/#2

megies added some commits May 18, 2017

testing: subclass unittest.TestCase and plug in AttribDict+Stats
comparison with useful assertion error message
@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 18, 2017

Member

Wohoo :) While at it: can you also drop the u prefix for unicode strings in Python 2 in the pretty representation?

Done. Pretty ugly (sic!), but didn't see an immediate better way to do it..

Member

megies commented May 18, 2017

Wohoo :) While at it: can you also drop the u prefix for unicode strings in Python 2 in the pretty representation?

Done. Pretty ugly (sic!), but didn't see an immediate better way to do it..

@megies megies referenced this pull request May 18, 2017

Merged

fix current network test fails for release of 1.1.0 #1791

1 of 2 tasks complete

megies added some commits May 18, 2017

Stats/AttribDict pretty printing: make sure that list of priorized keys
which is a class attribute will never be modified in place
@@ -166,9 +166,6 @@ def __str__(self):
if self.decimation_input_sample_rate is not None else ""))
return ret.strip()
def _repr_pretty_(self, p, cycle):

This comment has been minimized.

@QuLogic

QuLogic May 18, 2017

Member

Did this get moved down a layer or something? I'm not sure how this relates to AttribDict.

@QuLogic

QuLogic May 18, 2017

Member

Did this get moved down a layer or something? I'm not sure how this relates to AttribDict.

This comment has been minimized.

@megies

megies May 22, 2017

Member

This doesn't relate to AttribDict, just another place that can use some polishing. I've moved _repr_pretty_ to the base class ComparingObject and also enhanced it some.

@megies

megies May 22, 2017

Member

This doesn't relate to AttribDict, just another place that can use some polishing. I've moved _repr_pretty_ to the base class ComparingObject and also enhanced it some.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 22, 2017

Member

@krischer this is changing dockerfiles btw, so docker images would have to be re-created once this is merged into master so that the new test runs.

Member

megies commented May 22, 2017

@krischer this is changing dockerfiles btw, so docker images would have to be re-created once this is merged into master so that the new test runs.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 24, 2017

Member

If nobody'll complain I'll merge this in a couple of days.

Member

megies commented May 24, 2017

If nobody'll complain I'll merge this in a couple of days.

@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer May 24, 2017

Member

Does this really need to depend on IPython for the tests? Could we not just call _repr_pretty() ourselves with some mock objects? IMHO definitely worth it in order to avoid another test dependency.

Also I don't fully understand why _repr_pretty() does not call __str__() under the hood like for some of the other objects. But I might just be missing something here.

Last I'd also rather avoid subclassing unittest.TestCast as it will make it harder to eventually move to pytest which I guess we should in the long run. The attrib dict compare function could just be a function in obspy.core.util.testing.

Member

krischer commented May 24, 2017

Does this really need to depend on IPython for the tests? Could we not just call _repr_pretty() ourselves with some mock objects? IMHO definitely worth it in order to avoid another test dependency.

Also I don't fully understand why _repr_pretty() does not call __str__() under the hood like for some of the other objects. But I might just be missing something here.

Last I'd also rather avoid subclassing unittest.TestCast as it will make it harder to eventually move to pytest which I guess we should in the long run. The attrib dict compare function could just be a function in obspy.core.util.testing.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 24, 2017

Member

Also I don't fully understand why _repr_pretty() does not call str() under the hood like for some of the other objects. But I might just be missing something here.

Might be another way to do it but from what I understood, using the pretty printer and especially nested groups is needed to get nested levels of indenting right.

Could we not just call _repr_pretty() ourselves with some mock objects?

I guess so, but mocking the ipython pretty printer object will mean quite some work..

Last I'd also rather avoid subclassing unittest.TestCast as it will make it harder to eventually move to pytest which I guess we should in the long run.

I really don't see how this tiny change would make the switch harder. The upside of this approach (registering with TestCase) is that it will immediately work in all places where to Stats objects are compared, without explicitly searching through all test code for places that actually compare two stats objects. The worst thing that could happen is that we lose that pretty error message again during transition to pytest. If I change it to a specialized function it won't be able to catch all Stats/Stats or AttribDict/AttribDict comparisons in our whole test code base.

OK, so I'll bump this to a later release..

Member

megies commented May 24, 2017

Also I don't fully understand why _repr_pretty() does not call str() under the hood like for some of the other objects. But I might just be missing something here.

Might be another way to do it but from what I understood, using the pretty printer and especially nested groups is needed to get nested levels of indenting right.

Could we not just call _repr_pretty() ourselves with some mock objects?

I guess so, but mocking the ipython pretty printer object will mean quite some work..

Last I'd also rather avoid subclassing unittest.TestCast as it will make it harder to eventually move to pytest which I guess we should in the long run.

I really don't see how this tiny change would make the switch harder. The upside of this approach (registering with TestCase) is that it will immediately work in all places where to Stats objects are compared, without explicitly searching through all test code for places that actually compare two stats objects. The worst thing that could happen is that we lose that pretty error message again during transition to pytest. If I change it to a specialized function it won't be able to catch all Stats/Stats or AttribDict/AttribDict comparisons in our whole test code base.

OK, so I'll bump this to a later release..

@megies megies modified the milestones: 1.2.0, 1.1.0 May 24, 2017

@megies megies modified the milestones: 1.2.0, 1.3.0 Apr 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment