Skip to content
This repository has been archived by the owner on May 4, 2020. It is now read-only.

Allows get sheet or sheet index by sheet name #92

Merged
merged 6 commits into from Nov 21, 2016

Conversation

fabiofsilva
Copy link
Contributor

Hi,
This pull request is referent to #66, that was closed due lack of feedback. I've made the changes that you requested. Sorry, but I was working in another project.

@coveralls
Copy link

coveralls commented Oct 6, 2016

Coverage Status

Coverage increased (+0.06%) to 58.635% when pulling c745102 on fabiofsilva:master into 917a8ad on python-excel:master.

try:
self.wb.get_sheet(1.1)
except Exception as e:
self.assertTrue('sheet must be integer or string', e)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't asserting what you think it's asserting...
(hint: put e=None on the line above...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I did not understand what you mean by putting e=None. Why do you think is not asserting?

Copy link
Member

Choose a reason for hiding this comment

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

Then please go and read the documentation for assertTrue so you understand why it's the wrogn assertion method to use here. Hint: your current assertion will always pass. When you write tests, make sure they fail before fixing the code...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You where rigth, my mistake :). I was trying to test the exception message. I'll test only if the exception is raised then.

@coveralls
Copy link

coveralls commented Oct 10, 2016

Coverage Status

Coverage increased (+0.06%) to 58.635% when pulling 3319144 on fabiofsilva:master into 917a8ad on python-excel:master.

Copy link
Member

@sjmachin sjmachin left a comment

Choose a reason for hiding this comment

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

in test for get sheet by name, arg is an index
in test for get sheet by index, arg is a sheet name
please explain and/or fix

@fabiofsilva
Copy link
Contributor Author

Fixed on last commit.

@cjw296 cjw296 merged commit bf23e1c into python-excel:master Nov 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants