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

Displaying Compound model expressions when printing compound model instances #4482

Merged
merged 3 commits into from Jan 14, 2016

Conversation

Projects
None yet
2 participants
@anchitjain1234
Contributor

anchitjain1234 commented Jan 13, 2016

Fix for #4414 .
Example

In [23]:  g1 = models.Gaussian1D(1, 0, 0.2)

In [24]:  g2 = models.Gaussian1D(2.5, 0.5, 0.1)

In [25]:  gg = g1 + g2

In [26]:  print(gg)
Model: CompoundModel0
Inputs: (u'x',)
Outputs: (u'y',)
Model set size: 1
Expression: [0] + [1]
Components: 
    [0]: <Gaussian1D(amplitude=1.0, mean=0.0, stddev=0.2)>

    [1]: <Gaussian1D(amplitude=2.5, mean=0.5, stddev=0.1)>
Parameters:
    amplitude_0 mean_0 stddev_0 amplitude_1 mean_1 stddev_1
    ----------- ------ -------- ----------- ------ --------
            1.0    0.0      0.2         2.5    0.5      0.1
@embray

This comment has been minimized.

Show comment
Hide comment
@embray

embray Jan 13, 2016

Member

@anchitjain1234 This looks great. I have a few minor nitpicks with how I'd like the code refactored, and I'll get back to you in a bit with that. But otherwise this looks correct.

Member

embray commented Jan 13, 2016

@anchitjain1234 This looks great. I have a few minor nitpicks with how I'd like the code refactored, and I'll get back to you in a bit with that. But otherwise this looks correct.

@embray embray closed this Jan 13, 2016

@embray embray reopened this Jan 13, 2016

@embray

This comment has been minimized.

Show comment
Hide comment
@embray

embray Jan 13, 2016

Member

(Didn't mean to close)

Member

embray commented Jan 13, 2016

(Didn't mean to close)

Show outdated Hide outdated astropy/modeling/core.py
# This case is mostly for debugging purposes
return cls._format_cls_repr()
def _get_expr_comp(cls):

This comment has been minimized.

@embray

embray Jan 13, 2016

Member

Instead, just call this _format_components and have it return the components part--not the list of keywords tuples. Leave the part that builds the list of keywords in __repr__ where it was. Same in __str__ below. In some sense that actually has a little bit more code duplication, but uses the interfaces more clearly I think, and will be easier to extend later if needed.

Also move this method lower in the class--I've tried to consistently keep all "internal" methods toward the end of classes in this module.

@embray

embray Jan 13, 2016

Member

Instead, just call this _format_components and have it return the components part--not the list of keywords tuples. Leave the part that builds the list of keywords in __repr__ where it was. Same in __str__ below. In some sense that actually has a little bit more code duplication, but uses the interfaces more clearly I think, and will be easier to extend later if needed.

Also move this method lower in the class--I've tried to consistently keep all "internal" methods toward the end of classes in this module.

Show outdated Hide outdated astropy/modeling/core.py
@@ -2433,6 +2436,10 @@ class _CompoundModel(Model):
_submodels = None
def __str__(self):
keywords=self._get_expr_comp()
return super(_CompoundModel,self)._format_str(keywords=keywords)

This comment has been minimized.

@embray

embray Jan 13, 2016

Member

Should be a space after the comma here.

@embray

embray Jan 13, 2016

Member

Should be a space after the comma here.

Show outdated Hide outdated astropy/modeling/core.py
@@ -2433,6 +2436,10 @@ class _CompoundModel(Model):
_submodels = None
def __str__(self):
keywords=self._get_expr_comp()

This comment has been minimized.

@embray

embray Jan 13, 2016

Member

Should be spaces around the = here (but not around the = in the method call below).

@embray

embray Jan 13, 2016

Member

Should be spaces around the = here (but not around the = in the method call below).

Show outdated Hide outdated CHANGES.rst
@@ -150,6 +150,9 @@ Bug fixes
- ``astropy.modeling``
- Displaying Compound model expressions when printing compound model instances
. [#4414]

This comment has been minimized.

@embray

embray Jan 13, 2016

Member

This is a sentence fragment. It should be something like
"Fixed display of compound model expressions and components when printing compound model instances. [#4414]"

@embray

embray Jan 13, 2016

Member

This is a sentence fragment. It should be something like
"Fixed display of compound model expressions and components when printing compound model instances. [#4414]"

@embray

This comment has been minimized.

Show comment
Hide comment
@embray

embray Jan 13, 2016

Member

All of the above are minor nitpicks that should be addressed. Thanks @anchitjain1234 👍

Member

embray commented Jan 13, 2016

All of the above are minor nitpicks that should be addressed. Thanks @anchitjain1234 👍

@anchitjain1234

This comment has been minimized.

Show comment
Hide comment
@anchitjain1234

anchitjain1234 Jan 14, 2016

Contributor

Is this OK @embray ?

Contributor

anchitjain1234 commented Jan 14, 2016

Is this OK @embray ?

@embray

This comment has been minimized.

Show comment
Hide comment
@embray

embray Jan 14, 2016

Member

Perfect, thanks!

Member

embray commented Jan 14, 2016

Perfect, thanks!

@embray embray added this to the v1.0.9 milestone Jan 14, 2016

embray added a commit that referenced this pull request Jan 14, 2016

Merge pull request #4482 from anchitjain1234/fix-4414
Displaying Compound model expressions when printing compound model instances

@embray embray merged commit 80af955 into astropy:master Jan 14, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.004%) to 76.567%
Details

@anchitjain1234 anchitjain1234 deleted the anchitjain1234:fix-4414 branch Jan 14, 2016

embray added a commit that referenced this pull request Jan 19, 2016

Merge pull request #4482 from anchitjain1234/fix-4414
Displaying Compound model expressions when printing compound model instances

embray added a commit that referenced this pull request Jan 19, 2016

embray added a commit that referenced this pull request Jan 19, 2016

Merge pull request #4482 from anchitjain1234/fix-4414
Displaying Compound model expressions when printing compound model instances

embray added a commit that referenced this pull request Jan 27, 2016

Merge pull request #4482 from anchitjain1234/fix-4414
Displaying Compound model expressions when printing compound model instances

embray added a commit that referenced this pull request Jan 27, 2016

Merge pull request #4482 from anchitjain1234/fix-4414
Displaying Compound model expressions when printing compound model instances

dhomeier added a commit to dhomeier/astropy that referenced this pull request Jun 12, 2016

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