Skip to content

Conversation

jgaffiot
Copy link

The first commit modifies only 1 line to fix the issue #352.
The second commit homogenizes the __repr__ method using str.format, and also uses format on some other strings and removes unnecessary concatenation.

@wholmgren
Copy link
Member

Looks good, thanks.

Let's add a test to make sure this doesn't happen again. I think the code below can replace test_modelchain.py lines 420-443:

@pytest.mark.parametrize('strategy, strategy_str', [
    ('south_at_latitude_tilt', 'south_at_latitude_tilt'),
    (None, 'None')])  # GitHub issue 352
def test_ModelChain___repr__(system, location, strategy, strategy_str):

    mc = ModelChain(system, location, orientation_strategy=strategy,
                    name='my mc')

    expected = '\n'.join([
        'ModelChain: ',
        '  name: my mc',
        '  orientation_strategy: ' + strategy_str,
        '  clearsky_model: ineichen',
        '  transposition_model: haydavies',
        '  solar_position_method: nrel_numpy',
        '  airmass_model: kastenyoung1989',
        '  dc_model: sapm',
        '  ac_model: snlinverter',
        '  aoi_model: sapm_aoi_loss',
        '  spectral_model: sapm_spectral_loss',
        '  temp_model: sapm_temp',
        '  losses_model: no_extra_losses'
    ])

    assert mc.__repr__() == expected

Please also add a note and your name to the whatsnew file in docs/sphinx/source/whatsnew/v0.4.6.rst

@wholmgren wholmgren added this to the 0.4.6 milestone Jul 25, 2017
@wholmgren wholmgren added the bug label Jul 25, 2017
@jgaffiot
Copy link
Author

I've updated the pull request with your code and the update of whatsnew/v0.4.6.rst

@wholmgren
Copy link
Member

Great, thanks @jgaffiot!

@wholmgren wholmgren merged commit 289e473 into pvlib:master Jul 26, 2017
@wholmgren wholmgren modified the milestones: 0.4.6, 0.5.0 Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants