-
Notifications
You must be signed in to change notification settings - Fork 37
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
Updated DQR tool to use new DQR web-service. #743
Conversation
@kenkehoe Code wise, things look good! However looks to be an error now with one of the examples: that's failing due to a missing variable |
Ummmm, OK? I'll take a look. |
So it appears the example was using the call to add_dqr_to_qc() as a function to add a QC variable that does not exists in the Dataset. I don't see any DQRs for the period of data in testing. Should we update the functionality to create an empty QC variable to match the current code? Seems a little strange but I could put in a check to continue this ability of use. Would only force the addition of qc variable for variables keyword. |
@kenkehoe Hmm yeah i'm not to sure, @AdamTheisen thoughts? |
@kenkehoe I'm not sure I'm following you. The example uses the AOSMET that has a DQR on it from this example right? https://arm-doe.github.io/ACT/source/auto_examples/qc/plot_dqr_qc.html#sphx-glr-source-auto-examples-qc-plot-dqr-qc-py |
Ummm, OK? I did a search for MAR aosmet on the DQR Web-service and no DQR comes up. When I query the new DQR web-service it returns an object suggesting no DQR exists. https://task.arm.gov/dqr/#s/site.keyword::MAR&iclass.keyword::aosmet&_r::_ https://dqr-web-service.svcs.arm.gov/dqr_full/maraosmetM1.a1/20180201/20180201/incorrect,suspect But the DQR does appear to exist? So something is not quite right. But along the documentation in the example it does insinuate that the QC variable will be created. |
Hold on, there is something strange with the date range and the new web-service. I have a question in to James about it. |
@kenkehoe that's strange. I see 6 DQRs for AOSMET from the link you posted so the new web service does seem to be problematic in what it's returning. But yes. I do think it's valuable to create the QC variable as part of this call if no QC variable exists right? I think the AOSMET is the use case for when we want to apply DQRs to something with no QC variables in the file. THoughts? |
@AdamTheisen OK the code will create a new QC variable if one does not exist when it finds a DQR to apply. I just added some additional code to make the QC variable regardless of the result of the DQR web-service. It will create the missing QC variable if the variable keyword is provided. Is that the function we want to do? Could be confusing to have an empty QC variable, and I don't know how all the downstream tools will handle an empty QC variable. Something is not right with the web-service and submitting dates. I think they subset the dates based on the date range in the query. When I search for one day only nothing is returned. But when I search over two days I get two date ranges. |
@kenkehoe that makes sense the way you're thinking. Don't create a variable unless there's a dqr |
Can we rerun this. The web-service has been updated/fixed. |
@mgrover1 @zssherman this looks like an issue with micromamba and the CI right? |
@AdamTheisen yeah, they removed the python version argument - I am looking into this this morning |
I am attempting to fix it with #745 ... that should fix it, depending... |
As a note, we have the issue tagged in V2.0.0 but I have no issues with this going in sooner since it won't necessarily break anything that I'm aware of. |
The fix worked :) |
Merging |
This update will use the new ARM DQR web-service for applying DQRs. I updated to testing to ensure the keywords are used correctly.