CADWRDeltaModeling / dms_datastore

Data download and management tools for continuous data for Pandas. See documentation https://cadwrdeltamodeling.github.io/dms_datastore/
https://cadwrdeltamodeling.github.io/dms_datastore/
MIT License
1 stars 0 forks source link

setup.py requirements list is incomplete #1

Open dwr-psandhu opened 11 months ago

dwr-psandhu commented 11 months ago

Description

conda installed a new environment as indicated by setup.py requirements.

Then pip install -e further installs other dependencies not listed.

Finally schimpy is not installed either way but populate_repo depends upon it

dwr-psandhu commented 11 months ago

schimpy dependency is also causing issues with building an environment. Especially now with pandas>2 (see issue #5). Excluding schismpy allows the environment to build, however there are dependencies on schimpy

@kjnam @esatel @HansKimDWR Any ideas on how to remove these dependencies? perhaps move some functions needed over to dms_datastore ?

kjnam commented 11 months ago

@dwr-psandhu I will look into it, but I think we want to adopt Pandas ver 2 in other packages that depend on each other.

water-e commented 11 months ago

@dwr-psandhu I think you are talking about a de facto circular dependencies caused by schimpy calling dms-datastore functions and vice versa. My only doubt is that you said "building an environment". Do you import to do the build?

Regardless it is a module-level circular dependency. There are two independent issues:

1) numerous calls to unit_conversions. I suggest we move these to vtools rather than dms_datastore. I prefer schimpy and dms_datastore don't know about each other and units is fine grain and should be shared between them. My long term hope for unit_conversions is to move away from hand-rolled units and instead build helpers around cf-units. We can talk about that another time. Any move will break a lot of existing code, but there is nothing otherwise hard or tricky so feel free to do that.

2) there is also one call to schimpy.station in populate repo. I'll refactor that. I'd rather duplicate something than create overlap with a single function.

There are other reasons folks should not use populate_repo quite yet unless we discuss it. It really ready without a lot of undocumented secret sausce. To make the build and dms_datastore safer I put a "SAFEGUARD" fence in the file that will cause functions in that one module to raise a NotImplementedError and to wall off the offending import.

water-e commented 6 months ago

This seems to have stalled. The unit_conversions got fixed a long time ago. I'll try removing the stations reference from schimpy.