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.

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

Fix single-item device list throwing an error #133

Merged
merged 1 commit into from
May 24, 2023

Conversation

MaciejSkrabski
Copy link
Contributor

Fix single item device list throwing an error

I noticed that if I pass a list containing a single item as device to any model that derives from BaseModel it results in an AssertionError, which I think is erroneous.
AssertionError: The list of devices should have at least 1 device, but got 0.

I test if the device variable is one item long. If it is, I extract the object it contains and rerun self._setup_device recursively.

Before submitting

  • This PR is made to fix a typo or improve the docs (you can dismiss the other checks if this is the case).
  • Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have written necessary tests and already run them locally.

Fix BaseModel-derived models throwing errors on single-item device list.
@coveralls
Copy link
Collaborator

coveralls commented May 23, 2023

Pull Request Test Coverage Report for Build 5062898256

  • 1 of 5 (20.0%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.01%) to 84.034%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pypots/base.py 1 5 20.0%
Files with Coverage Reduction New Missed Lines %
pypots/base.py 1 70.68%
pypots/cli/env.py 4 61.22%
Totals Coverage Status
Change from base Build 5055662432: -0.01%
Covered Lines: 3100
Relevant Lines: 3689

💛 - Coveralls

@WenjieDu
Copy link
Owner

Cool! I've fixed this error but haven't push my commits to GitHub for a merge. Congrats, Maciej! 😉 You're one step ahead of me. Let's merge it.

@WenjieDu WenjieDu merged commit b6dd4d9 into WenjieDu:dev May 24, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants