Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle no output #264

Merged
merged 7 commits into from Oct 21, 2019
Merged

Handle no output #264

merged 7 commits into from Oct 21, 2019

Conversation

lewisjared
Copy link
Collaborator

@lewisjared lewisjared commented Sep 23, 2019 •
edited

Allows an empty MAGICCData object to be returned from run (also ran make black)

  • Tests added
  • Description in CHANGELOG.rst added

znicholls
znicholls reviewed Sep 23, 2019
View reviewed changes
Copy link
Collaborator

@znicholls znicholls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Is it possible to test with MAGICC6 (if you have out_X=0 for everything)?

znicholls
znicholls reviewed Sep 23, 2019
View reviewed changes
tests/test_core.py Outdated Show resolved Hide resolved
znicholls
znicholls reviewed Sep 23, 2019
View reviewed changes
tests/test_core.py Outdated Show resolved Hide resolved
@znicholls
Copy link
Collaborator

Sorry @lewisjared just thinking about this more. Could we have the case of returning empty be a special case (e.g. only="empty")? I'm just thinking that returning empty dataframes is a special case, and for most users it would be better to raise an error message if there's no output. Option b would be that if not strict then returning empty works, otherwise it raises the error (in line with strict being for advanced users).

@lewisjared
Copy link
Collaborator Author

I don't like only="empty", but option b would work

@lewisjared
Copy link
Collaborator Author

Added a check for strict

@lewisjared
Copy link
Collaborator Author

And appveyor is broken

@znicholls znicholls mentioned this pull request Oct 21, 2019
Merged
@znicholls
Copy link
Collaborator

cool rebase then we're good

lewisjared and others added 7 commits October 21, 2019 16:15
@lewisjared
Improved handling of empty outputs
ecebc0c
@lewisjared
make black
4f46153
@lewisjared
changelog
519be68
@lewisjared
Use package to test both magicc6 and 7
cb75f26
@lewisjared @znicholls
Update tests/test_core.py
faa91ad 
Co-Authored-By: Zeb Nicholls <zebedee.nicholls@climate-energy-college.org>
@lewisjared @znicholls
Update tests/test_core.py
ec4aaf1 
Co-Authored-By: Zeb Nicholls <zebedee.nicholls@climate-energy-college.org>
@lewisjared
Only raise exception with strict
d2ebcef
@znicholls znicholls merged commit 9eac1b3 into master Oct 21, 2019
@znicholls znicholls deleted the handle-empty branch October 30, 2019 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@znicholls znicholls znicholls left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lewisjared @znicholls