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

iesave: error when more than one variable is used in idvars #347

Closed
luizaandrade opened this issue Oct 11, 2023 · 7 comments
Closed

iesave: error when more than one variable is used in idvars #347

luizaandrade opened this issue Oct 11, 2023 · 7 comments
Labels
minor bug Bug unlikely to lead to incorrect analysis resolved but not yet published Issue is fixed, but not yet published on SSC
Milestone

Comments

@luizaandrade
Copy link
Collaborator

If more than one variable is used in idvars and one of them has missing values, the code breaks on the following lines:

- if _rc {
  - capture assert !missing(`idvars')
  = capture assert !missing(user_uuid session)
  - if _rc {
  - count if missing(`idvars')
  = count if missing(user_uuid session)

The error I see is

user_uuidsession not found

I think this is because missing() doesn't take multiple variables as input.

@luizaandrade luizaandrade added the minor bug Bug unlikely to lead to incorrect analysis label Oct 11, 2023
@kbjarkefur
Copy link
Contributor

I was able to reproduce this error using this small example:

* Example generated by -dataex-. For more info, type help dataex
clear
input float(varA varB) str9 varC
1 1 "some data"
1 2 "some data"
1 . "some data"
1 4 "some data"
1 5 "some data"
2 1 "some data"
2 2 "some data"
2 3 "some data"
. 4 "some data"
2 5 "some data"
end
    
iesave "~/delete-out/issue347.dta", idvars(varA varB) version(15)

idvars() option should be able to handle multiple variables, so this is a bug.

@luizaandrade , just to make sure that we are on the same page. Missing values in either of these variables should still be an error right? But an error that the command handles and provide useful information about instead of the current cryptic message.

@kbjarkefur
Copy link
Contributor

Pushed a fix. It still generates and error, but now, following a non-zero return code fromcap isid `idvars', the command tests one idvar at the time. This is the output if two idvars are used and both of them have missing values.

image

@kbjarkefur kbjarkefur added the resolved but not yet published Issue is fixed, but not yet published on SSC label Oct 11, 2023
@kbjarkefur
Copy link
Contributor

Do you have time to look at #348 before I merge it?

@luizaandrade
Copy link
Collaborator Author

That was quick! Yes, I agree it should return an error when these are missing

@luizaandrade
Copy link
Collaborator Author

Do you have time to look at #348 before I merge it?

Yes, I can take a look at it this week

@kbjarkefur
Copy link
Contributor

We are about to publish a new version of ietoolkit. Did you have time to test this?

luizaandrade added a commit that referenced this issue Oct 28, 2023
@kbjarkefur
Copy link
Contributor

merged to develop in #348

@ankritisingh ankritisingh added this to the v7.3 milestone Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor bug Bug unlikely to lead to incorrect analysis resolved but not yet published Issue is fixed, but not yet published on SSC
Projects
None yet
Development

No branches or pull requests

3 participants