Closed JessicaS11 closed 7 months ago
:point_left: Launch a binder notebook on this branch for commit 2594153203f2974ccfc9ece95566d9493f4ea40f
I will automatically update this comment whenever this PR is modified
:point_left: Launch a binder notebook on this branch for commit 8dfb33e85565db4e757751386439a9a1855c4361
:point_left: Launch a binder notebook on this branch for commit d43da757b5e98d67818ee87a104d3ed43dd3d637
:point_left: Launch a binder notebook on this branch for commit fd18b74b6808fe78e9546edd9a186d370e43504d
:point_left: Launch a binder notebook on this branch for commit 9cc004058b0ae7bd368eca54a63f5187ff2f0db7
:point_left: Launch a binder notebook on this branch for commit d877e8b1e509d507cd4ae385c30b4c3a04fa79ce
:point_left: Launch a binder notebook on this branch for commit f9b6d8103686f216a1ab1ad28f77fa533dfa39f4
:point_left: Launch a binder notebook on this branch for commit 8fcab13420588964aeadcff896df12b2491c19b9
:point_left: Launch a binder notebook on this branch for commit 630415a3aec284cec4e8f7a4639ee0cdd0119cc8
:point_left: Launch a binder notebook on this branch for commit c246543963532e959d18e02666d334e9d58138b1
:point_left: Launch a binder notebook on this branch for commit 363dad21994dd0852d5077093000949f2b6656b6
:point_left: Launch a binder notebook on this branch for commit 2cdf07e964d9561029df5308de1cd395c1f6c2dd
:point_left: Launch a binder notebook on this branch for commit a91c36074e1cf2de6e56179833c2e42c1197a25e
:point_left: Launch a binder notebook on this branch for commit cb367e178e7d9fcd8425af839afebb923e28dd5e
:point_left: Launch a binder notebook on this branch for commit 381092f5c93f0140255422307a0272f4a2703704
:point_left: Launch a binder notebook on this branch for commit 63e1b5727c20bb2d5969597ca04e989210207027
:point_left: Launch a binder notebook on this branch for commit 5b6c65b574845449e05b7948082ad4befdf3ef1c
:point_left: Launch a binder notebook on this branch for commit 37d19b63c10aa0ab675ae32ba3b372cefee414a8
:point_left: Launch a binder notebook on this branch for commit b064224988f921a76de5851ba858065fd8f27b0d
:point_left: Launch a binder notebook on this branch for commit 51c2e83a14ac615d93872804f0007e3a342ab33c
:point_left: Launch a binder notebook on this branch for commit 93867076b228bec7ca7b4832a5682662d14479bc
:point_left: Launch a binder notebook on this branch for commit 417929fbe8abcae8747fd5145159d368af684287
:point_left: Launch a binder notebook on this branch for commit b8bf4ea8a2e5c70a3e85fbdfc12942896b05acc8
:point_left: Launch a binder notebook on this branch for commit 5ff8c83ecfe02c1dd56b03006d645bfaf813701c
:point_left: Launch a binder notebook on this branch for commit 251bf0bd3271fb74a1b52c206eac135fa857f658
:point_left: Launch a binder notebook on this branch for commit f514619ec30b8a41fea62849daf12b263c173ab4
:point_left: Launch a binder notebook on this branch for commit 53831aae133918f92d6d279747db10acc33f207e
:point_left: Launch a binder notebook on this branch for commit a0c5acd341a141f0a89bf5276772d3c3171fa84d
:point_left: Launch a binder notebook on this branch for commit a326949d3a1e19e8a15a7cad49e4b5211a2f3e99
:point_left: Launch a binder notebook on this branch for commit cce8fa70ef98d4ad4758269e017df753c71bdfe6
:point_left: Launch a binder notebook on this branch for commit efcb16d938f98893f50654a864e43f4a9dd5f927
:point_left: Launch a binder notebook on this branch for commit af5d8b08520e93cf298a173496599309c2d8a07b
:point_left: Launch a binder notebook on this branch for commit ce29a55eab27e36ee8b5280947d0f0e93ef7c318
:point_left: Launch a binder notebook on this branch for commit 98579787f1610c1c960a7d5b55abac8464119917
:point_left: Launch a binder notebook on this branch for commit b99855e83be01da87bd11596ff203f05ecd9ddfd
:point_left: Launch a binder notebook on this branch for commit 9aaca81c56b3672b774834ca338038aaae6020dc
:point_left: Launch a binder notebook on this branch for commit f6eb1b2bab394ac3ddfd7e8d7bae0e2778f8af31
:point_left: Launch a binder notebook on this branch for commit e8f8c387735d4d2f2a3146a57d51859cef6bfa2d
:point_left: Launch a binder notebook on this branch for commit 6dac12c49b9a8823ad986aecee5ec23f424e90ef
:point_left: Launch a binder notebook on this branch for commit eea686c58e72be92f26ffe970503ebb3d679ac79
:point_left: Launch a binder notebook on this branch for commit 3fb4c48fd52a881933e872553d7101e4f18feb36
:point_left: Launch a binder notebook on this branch for commit 283fc04bfb722a9c15bd1d27eb0d6869840c89e4
:point_left: Launch a binder notebook on this branch for commit d888127f22efcaade481c7612eeba144164eb506
:point_left: Launch a binder notebook on this branch for commit ab4032834a99e44d43b2908b6cb374d6819d626c
:point_left: Launch a binder notebook on this branch for commit 2a9efe497ad84a590051d1a06a2e5f200bfd3dec
:point_left: Launch a binder notebook on this branch for commit 24de1bc0716b557dfd64d0625428c05b1b9af62e
:point_left: Launch a binder notebook on this branch for commit 36cac5792edb55633fbf1f8045d1574be1b0b315
:point_left: Launch a binder notebook on this branch for commit 74c3a6030fa95fd7a3ae1158e0aa5989be1d1cd7
:point_left: Launch a binder notebook on this branch for commit 79ecc27e81fbd87bd1710da6a093b88e48b4a8e6
:point_left: Launch a binder notebook on this branch for commit 377d3c07b4f580bda451ec7947047209e50f7ad5
:point_left: Launch a binder notebook on this branch for commit cc8b1d0025a1c9af9841f2d34c285f3fbe472d51
:point_left: Launch a binder notebook on this branch for commit d467a847452386f7bdd0e8e76559179eac544822
:point_left: Launch a binder notebook on this branch for commit fc9e086facc6f78548c3c1ca7880137fede0ebf6
:point_left: Launch a binder notebook on this branch for commit 54a14eab32c6707ff5b3163e36eba9d9ca5f1898
:point_left: Launch a binder notebook on this branch for commit ae1d484abeb5b2e8cb8e62831833b1a82582bb1d
:point_left: Launch a binder notebook on this branch for commit 468f3ba16773a92aa62e81a569c2da3cccee67eb
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Looks like the a couple of checks failed, so I am unable to see the latest build of the documentation. I'm looking at the docs on a previous build (#435) for now.
Topmost traceback in the error message:
Traceback (most recent call last):
File "/home/docs/checkouts/readthedocs.org/user_builds/icepyx/envs/427/lib/python3.10/site-packages/sphinx/config.py", line 332, in eval_config_file
exec(code, namespace)
File "/home/docs/checkouts/readthedocs.org/user_builds/icepyx/checkouts/427/doc/source/conf.py", line 20, in <module>
import icepyx
File "/home/docs/checkouts/readthedocs.org/user_builds/icepyx/checkouts/427/icepyx/__init__.py", line 1, in <module>
from icepyx.core.query import Query, GenQuery
File "/home/docs/checkouts/readthedocs.org/user_builds/icepyx/checkouts/427/icepyx/core/query.py", line 15, in <module>
from granules import Granules
ModuleNotFoundError: No module named 'granules'
Looks like the a couple of checks failed
Ugh. I know exactly what this is. @RomiP - I guess it doesn't work after all! Zach, the fix is to change the from granules import Granules
in line 15 of query.py back to from icepyx.core.granules import Granules
. I'm happy to do this, or if you want to put it in with some other docs changes you've made to see how the build looks go for it.
from icepyx.core.granules import Granules
I have it opened right now, so I can make the quick fix.
In trying to debug the failing tests, I noted there may be something funny going on with the presRange inputs throughout argo.py
. I discovered this because the test_presRange_input_param
test was passing if I ran only that test, but failing if it was run as part of the test suite (and failing with no profiles found from the API). We'll need to track down what's happening with the inputs given the multiple ways of running the code (e.g. with or without an explicit call to search).
In trying to debug the failing tests, I noted there may be something funny going on with the presRange inputs throughout
argo.py
Follow up: we're inconsistent with our handling of params
and presRange
throughout. Namely, we give a default of params=['temperature']
, and then if the user supplies params
to a function, sometimes we add it to self.params
and sometimes we don't, and we never remove (or provide a way to) items from the self.params
list. The case is somewhat similar for presRange
, with some slight differences since it's not mandatory and isn't potentially additive.
So some questions for going forward:
params
or presRange
via keyword arguments to the specific functions, or by modifying the properties directly (so update self.params
by doing something like argo_object.params = ['a','list','of','things']
)?self.params
variable (i.e. so they can remove items), we must always use the stored attributes as an input to search or download data (with consideration for the next point)?params
or presRange
argument, does it get ADDED to the existing values, or REPLACE them? Does the behavior change based on whether they are searching for or downloading data?It mostly boils down to how we want the user to be able to create and modify (or not) their inputs. I don't think there is a right or wrong answer here, and the changes/logic should be pretty straightforward to implement, but I'd like some input from @kelseybisson @RomiP @zachghiaccio before deciding which path to go down.
@JessicaS11 thank you for this!!
I like users modifying the properties directly by updating self.params
if we let the user modify their self.params variable (i.e. so they can remove items), we must always use the stored attributes as an input to search or download data (with consideration for the next point)?
yes, but if I recall there's a limit of searching for 2 at a time? so would need to loop through the list (I think this was already solved by you and Romina but not 100% sure).
if a user supplies a params or presRange argument, does it get ADDED to the existing values, or REPLACE them?
I think it should replace whatever was there by default.
No, in my my opinion. I think it's fair to ask a user to input an argument to search, and then if they don't like what comes up, they can re-search using different arguments to download.
@zachghiaccio @RomiP would love your thoughts!
Hi team,
Best, Zach
On Wed, Nov 8, 2023 at 6:28 PM Jessica Scheick @.***> wrote:
In trying to debug the failing tests, I noted there may be something funny going on with the presRange inputs throughout argo.py
Follow up: we're inconsistent with our handling of params and presRange throughout. Namely, we give a default of params=['temperature'], and then if the user supplies params to a function, sometimes we add it to self.params and sometimes we don't, and we never remove (or provide a way to) items from the self.params list. The case is somewhat similar for presRange, with some slight differences since it's not mandatory and isn't potentially additive.
So some questions for going forward:
- Should users specify changes to params or presRange via keyword arguments to the specific functions, or by modifying the properties directly (so update self.params by doing something like argo_object.params = ['a','list','of','things'])?
- if we let the user modify their self.params variable (i.e. so they can remove items), we must always use the stored attributes as an input to search or download data (with consideration for the next point)?
- if a user supplies a params or presRange argument, does it get ADDED to the existing values, or REPLACE them? Does the behavior change based on whether they are searching for or downloading data?
It mostly boils down to how we want the user to be able to create and modify (or not) their inputs. I don't think there is a right or wrong answer here, and the changes/logic should be pretty straightforward to implement, but I'd like some input from @kelseybisson https://github.com/kelseybisson @RomiP https://github.com/RomiP @zachghiaccio https://github.com/zachghiaccio before deciding which path to go down.
— Reply to this email directly, view it on GitHub https://github.com/icesat2py/icepyx/pull/427#issuecomment-1802890478, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALQ7B4XRPCKSMAV27T2GPQ3YDQIQJAVCNFSM6AAAAAAYKU2JVWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBSHA4TANBXHA . You are receiving this because you were mentioned.Message ID: @.***>
-- Zachary Fair he/him Postdoctoral Fellow at NASA Goddard Space Flight Center Sponsored Affiliate of the Department of Climate and Space Science and Engineering, University of Michigan
Ph.D. 2021, Climate and Space Science and Engineering University of Michigan
Thanks for your input, @zachghiaccio and @kelseybisson. I streamlined the params and presRange handling such that:
params
and presRange
properties (and in the former case, if they do it will check that they've set the properties to valid parameters)params
or presRange
kwarg to the search_data()
or download()
functions, the supplied values will replace the existing properties and be used for that search/download (so the previous values will be gone).keep_existing=True
; it will add whatever is in the new parameter/presRange list to whatever is already in the dataframe, but again the previous list of parameters will be replaced.@zachghiaccio, I left a few notes and code snippets in the notebook. Let me know if you'd like a hand with updating the sections affected by this change
@RomiP, I would love a second set of eyes on the implementation and tests
@zachghiaccio these are great - thank you! A few questions:
reg_a.download_all()
and it successfully downloaded the IS2 but not the Argo (even though the parameters were still set). It kept ending up in the except
saying dataset_name
wasn't defined. The Argo object thus had no data associated with it.
This PR will introduce the Argo (including BGC) retrieval functionality to QUEST.