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

Inform error message in composite for fhx with no scars #155

Closed
chguiterman opened this issue Aug 6, 2019 · 2 comments · Fixed by #156
Closed

Inform error message in composite for fhx with no scars #155

chguiterman opened this issue Aug 6, 2019 · 2 comments · Fixed by #156
Assignees
Labels
bug Something isn't working duplicate This issue or pull request already exists
Milestone

Comments

@chguiterman
Copy link
Contributor

If an fhx object (like from this file) contains no fire scars, composite throws an uninformative error:

Error in fix.by(by.x, x) : 'by' must specify a uniquely valid column

This issue should be caught, flagged with a warning, and the composite should return an empty fhx object as is done when there are no events to composite.

@brews brews self-assigned this Aug 7, 2019
@brews brews added duplicate This issue or pull request already exists bug Something isn't working labels Aug 7, 2019
@brews
Copy link
Member

brews commented Aug 7, 2019

Closing this issue because it is a duplicate of #131. This should be fixed in master and will be part of the v0.0.5 release.

With the fix in #131, composite() it just returns an empty composite, without warning or message.

Let me know if you have strong feelings otherwise and maybe we can toss a warning() or message() in there. Also comment back here if we're still having this issue.

@brews brews closed this as completed Aug 7, 2019
@brews brews added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Aug 7, 2019
@brews
Copy link
Member

brews commented Aug 7, 2019

Okay, I'm still getting the error with this file given as an example. I'm removing the duplicate tag and we'll see if we can't fix this for v0.0.5.

@brews brews reopened this Aug 7, 2019
@brews brews added this to the v0.5.0 milestone Aug 7, 2019
@brews brews closed this as completed in #156 Aug 7, 2019
brews added a commit that referenced this issue Aug 7, 2019
Fix bug in composite, add unit tests, close #155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants