search

podaac / l2ss-py

Level 2 subsetter with Harmony integration
https://podaac.github.io/l2ss-py/
Apache License 2.0
11 stars 12 forks source link

update ascending flag variable clean up #154

Closed nlenssen2013 closed 1 year ago

nlenssen2013 commented 1 year ago

Github Issue: #153

Description

xarray decode times chokes on blank ascending time variable

Overview of work done

remove the variable if the flag indicates it is blank

Overview of verification done

Open up three different files, ascending, descending and both

Overview of integration done

Explain how this change was integration tested. Provide screenshots or logs if appropriate. An example of this would be a local Harmony deployment.

PR checklist:

See Pull Request Review Checklist for pointers on reviewing this pull request

nlenssen2013 commented 1 year ago

Looks good Nick. Could you do me a favor though? Could you roll those dependabot updates into this PR? Bump werkzeug to 2.2.3, and cryptography to 39.0.1. It may need a poetry update, but not sure. Thanks!

Looks good Nick. Could you do me a favor though? Could you roll those dependabot updates into this PR? Bump werkzeug to 2.2.3, and cryptography to 39.0.1. It may need a poetry update, but not sure. Thanks!

Got it, so merge the other two pull requests into this one?

jamesfwood commented 1 year ago

Looks good Nick. Could you do me a favor though? Could you roll those dependabot updates into this PR? Bump werkzeug to 2.2.3, and cryptography to 39.0.1. It may need a poetry update, but not sure. Thanks!

Looks good Nick. Could you do me a favor though? Could you roll those dependabot updates into this PR? Bump werkzeug to 2.2.3, and cryptography to 39.0.1. It may need a poetry update, but not sure. Thanks!

Got it, so merge the other two pull requests into this one?

Yeah, or just update those versions and after it passes I can just close those tickets. Either way, up to you.

nlenssen2013 commented 1 year ago

Looks good Nick. Could you do me a favor though? Could you roll those dependabot updates into this PR? Bump werkzeug to 2.2.3, and cryptography to 39.0.1. It may need a poetry update, but not sure. Thanks!

Looks good Nick. Could you do me a favor though? Could you roll those dependabot updates into this PR? Bump werkzeug to 2.2.3, and cryptography to 39.0.1. It may need a poetry update, but not sure. Thanks!

Got it, so merge the other two pull requests into this one?

Yeah, or just update those versions and after it passes I can just close those tickets. Either way, up to you.

Okay, so I did poetry add werkzeug=='2.2.3' and poetry add cryptography=='39.0.1'. Then did a poetry update, pushed the changes in the lock and toml files. Seems like the poetry version is still wrong....

frankinspace commented 1 year ago

@nlenssen2013 you just need to run poetry update and then check in the new poetry.lock file that gets generated. I ran it and the updated version are there now.

nlenssen2013 commented 1 year ago

@nlenssen2013 you just need to run poetry update and then check in the new poetry.lock file that gets generated. I ran it and the updated version are there now.

Looks like it's still failing at the same spot

frankinspace commented 1 year ago

Didn't realize the action was also failing, that was a separate problem. Needed to bump the version of poetry the github action itself was using.

nlenssen2013 commented 1 year ago

Didn't realize the action was also failing, that was a separate problem. Needed to bump the version of poetry the github action itself was using.

You alright if I merge this?

frankinspace commented 1 year ago

Need to figure out why tests are failing

nlenssen2013 commented 1 year ago

Need to figure out why tests are failing

Will this get figured out this week?

jamesfwood commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

nlenssen2013 commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

jamesfwood commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

Ok, that would be great. Did you see the error in Github Actions? It's something with this: AttributeError: module 'numpy' has no attribute 'warnings'

Maybe your local version has a different version of numpy?

Thanks for checking it out! Also, sorry about the trouble with updating those two libraries. Didn't think it would cause so many problems.

nlenssen2013 commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

Ok, that would be great. Did you see the error in Github Actions? It's something with this: AttributeError: module 'numpy' has no attribute 'warnings'

Maybe your local version has a different version of numpy?

Thanks for checking it out! Also, sorry about the trouble with updating those two libraries. Didn't think it would cause so many problems.

Does numpy need to be 1.24? because everything goes through and passes when it remains numpy==1.23.5

jamesfwood commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

Ok, that would be great. Did you see the error in Github Actions? It's something with this: AttributeError: module 'numpy' has no attribute 'warnings' Maybe your local version has a different version of numpy? Thanks for checking it out! Also, sorry about the trouble with updating those two libraries. Didn't think it would cause so many problems.

Does numpy need to be 1.24? because everything goes through and passes when it remains numpy==1.23.5

yeah go ahead and downgrade numpy to 1.23.5 and push it and see if that works in github actions.

nlenssen2013 commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

Ok, that would be great. Did you see the error in Github Actions? It's something with this: AttributeError: module 'numpy' has no attribute 'warnings' Maybe your local version has a different version of numpy? Thanks for checking it out! Also, sorry about the trouble with updating those two libraries. Didn't think it would cause so many problems.

Does numpy need to be 1.24? because everything goes through and passes when it remains numpy==1.23.5

yeah go ahead and downgrade numpy to 1.23.5 and push it and see if that works in github actions.

nlenssen2013 commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

Ok, that would be great. Did you see the error in Github Actions? It's something with this: AttributeError: module 'numpy' has no attribute 'warnings' Maybe your local version has a different version of numpy? Thanks for checking it out! Also, sorry about the trouble with updating those two libraries. Didn't think it would cause so many problems.

Does numpy need to be 1.24? because everything goes through and passes when it remains numpy==1.23.5

yeah go ahead and downgrade numpy to 1.23.5 and push it and see if that works in github actions.

Was just trying to delete my comment - looks like sonar cloud isn't too happy

jamesfwood commented 1 year ago

@nlenssen2013 Would you have time to fix the tests or if not I can do it. Let me know, thanks!

I can work on it today, but tests are passing when I run it locally. I have the updated cryptography and werkzeug versions as well - so not sure where to trouble shoot

Ok, that would be great. Did you see the error in Github Actions? It's something with this: AttributeError: module 'numpy' has no attribute 'warnings' Maybe your local version has a different version of numpy? Thanks for checking it out! Also, sorry about the trouble with updating those two libraries. Didn't think it would cause so many problems.

Does numpy need to be 1.24? because everything goes through and passes when it remains numpy==1.23.5

yeah go ahead and downgrade numpy to 1.23.5 and push it and see if that works in github actions.

Was just trying to delete my comment - looks like sonar cloud isn't too happy

If you want to get this PR in, then go ahead and squash and merge it and we can deal with the sonar cloud issue another time. It doesn't seem like anything critical right now.