Changing packaging structure #5

aradhakrishnanGFDL · 2024-07-19T16:55:13Z

We need a sub-package structure, as in one can import just catalogs, or the wrapper scripts, or the API itself, or get all at once. But, also noting that going forward the catalogs perhaps will also be pushed to a schema repo.

Given the landscape change, we are revisiting the way packaging was done and see if technical issues can be resolved to get this done, collectively.

check for failure points without removing it all

checking for failure points

aradhakrishnanGFDL · 2024-07-19T20:02:11Z

I've opened a couple issues for those that could be addressed in a different PR so this is a bit manageable to review. Appreciate quick improvements and fixes needed to address this PR.. while the bigger ones can be new "issues" to address later.

ilaflott · 2024-07-22T17:55:24Z

@ilaflott with the changes in the PR, we should be able to the following

from catalogbuilder.scripts import gen_intake_gfdl

I can confirm this in a clean environment but i am getting a warning with a fresh code checkout:

Python 3.12.4 | packaged by conda-forge | (main, Jun 17 2024, 10:23:07) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import catalogbuilder
>>> from catalogbuilder.scripts import gen_intake_gfdl
The module intakebuilder is not installed. Do you have intakebuilder in your sys.path or have you activated the conda environment with the intakebuilder package in it?
Attempting again with adjusted sys.path

which brings me to these lines that i find confusing. it does seem to work though.

and hopefully be able to do the following

conda install noaa-gfdl::catalogbuilder
and bonus conda install catalogbuilder.scripts -c noaa-gfdl

happy to test that out upon publishing pipeline, which runs on merge i take it?

Someone needs to remove the intakebuilder package once this PR is approved, and communicate to ensure analysis-scripts is updated.

@ilaflott Do you know how to update the CONDA tokens (if GitHub teams allow)? See a silent failure here

sorry i haven't touched github secrets and/or conda tokens. I am unsure if i even have permissions, we can chat elsewhere about this if we need.

ilaflott · 2024-07-22T17:56:45Z

I pushed a super small tweak to the reqs detailed in the meta.yaml and setup.py

ilaflott · 2024-07-22T18:18:57Z

@Ciheim i think you can set the secrets for the respository? https://github.com/orgs/NOAA-GFDL/teams/catalogbuilder-admins

aradhakrishnanGFDL · 2024-07-22T18:22:44Z

@ilaflott with the changes in the PR, we should be able to the following
from catalogbuilder.scripts import gen_intake_gfdl

I can confirm this in a clean environment but i am getting a warning with a fresh code checkout:
Python 3.12.4 | packaged by conda-forge | (main, Jun 17 2024, 10:23:07) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import catalogbuilder
>>> from catalogbuilder.scripts import gen_intake_gfdl
The module intakebuilder is not installed. Do you have intakebuilder in your sys.path or have you activated the conda environment with the intakebuilder package in it?
Attempting again with adjusted sys.path
which brings me to these lines that i find confusing. it does seem to work though.

Right, if the conda package is installed and user activates the environment, you won't see this error. Since users have an option to run the catalog builder without the conda package, we expect them to add the necessary system paths so we can find the intakebuilder. We do need a new CI test that tests the conda package upon publishing (also a GitHub issue, though unsure when it will be taken up)

and hopefully be able to do the following
conda install noaa-gfdl::catalogbuilder
and bonus conda install catalogbuilder.scripts -c noaa-gfdl

happy to test that out upon publishing pipeline, which runs on merge i take it?

Someone needs to remove the intakebuilder package once this PR is approved, and communicate to ensure analysis-scripts is updated.
@ilaflott Do you know how to update the CONDA tokens (if GitHub teams allow)? See a silent failure here

sorry i haven't touched github secrets and/or conda tokens. I am unsure if i even have permissions, we can chat elsewhere about this if we need.

I've added the secret. The conda publish CI pipeline is now working as expected per the logs. Though it's not approved - I did push the package to the noaa-gfdl channel through this CI (have issues opened, we need a test channel or something too later). So conda install could be tested now.

aradhakrishnanGFDL · 2024-07-22T18:23:42Z

@ilaflott https://anaconda.org/noaa-gfdl/catalogbuilder is now up!

ilaflott · 2024-07-22T19:46:34Z

so conda install noaa-gfdl::catalogbuilder works, but...

(ians_CLEAN_env) ldt-4788479: ~ $] python
Python 3.12.4 | packaged by conda-forge | (main, Jun 17 2024, 10:23:07) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import catalogbuilder
>>> from catalogbuilder.scripts import gen_intake_gfdl
The module intakebuilder is not installed. Do you have intakebuilder in your sys.path or have you activated the conda environment with the intakebuilder package in it?
Attempting again with adjusted sys.path

now i look back at those import lines we WANT to work, in the try blocks:

from intakebuilder import gfdlcrawler, CSVwriter, builderconfig, configparser

this won't work because the package is no longer called intakebuilder, it's called catalogbuilder, and these pieces are under catalogbuilder.intakebuilder here. that's why python can't find those scripts and why the dirname(dirname(filename))) output getting added to sys.path works to solve the import issue- it's essentially saying "look for the scripts at the root directory of this package"

If we adjust the import calls, we can avoid sys.path manipulation, change the line to instead:

from catalogbuilder.intakebuilder import gfdlcrawler, CSVwriter, builderconfig, configparser

...or i can even use a relative import:

from . import gfdlcrawler, CSVwriter, builderconfig, configparser

... and changing the same import lines in applicable files within catalogbuilder/intakebuilder and we get the desired behavior, no warning.

ilaflott · 2024-07-22T19:48:59Z

not sure conda install catalogbuilder.scripts -c noaa-gfdl its working as excpected. but that being said, the package is quite small anyways, so i dont think the incentive to get this installation mode working is huge. The main approach, conda install noaa-gfdl::catalogbuilder works well and is more important.

(ians_CLEAN_env) ldt-4788479: ~ $] mamba install catalogbuilder.scripts -c noaa-gfdl

Looking for: ['catalogbuilder.scripts']

conda-forge/linux-64                                        Using cache
conda-forge/noarch                                          Using cache
r/linux-64                                                  Using cache
r/noarch                                                    Using cache
noaa-gfdl/linux-64                                            No change
noaa-gfdl/noarch                                              No change
main/linux-64                                                 No change
main/noarch                                                   No change
Could not solve for environment specs
The following package could not be installed
└─ catalogbuilder.scripts does not exist (perhaps a typo or a missing channel).
(ians_CLEAN_env) ldt-4788479: ~ $] conda install catalogbuilder.scripts -c noaa-gfdl
Channels:
 - noaa-gfdl
 - defaults
Platform: linux-64
Collecting package metadata (repodata.json): done
Solving environment: failed

PackagesNotFoundError: The following packages are not available from current channels:

  - catalogbuilder.scripts

Current channels:

  - https://conda.anaconda.org/noaa-gfdl
  - defaults
  - https://conda.anaconda.org/conda-forge

To search for alternate channels that may provide the conda package you're
looking for, navigate to

    https://anaconda.org

and use the search bar at the top of the page.

`catalogbuilder.intakebuilder` not found in pipeline. b.c. the package isnt installed into the created environment. adding `pip install .` to check

…like one always expects? .... wouldnt totally suprise me.

ilaflott · 2024-07-22T20:47:24Z

Well i certainly see the challenge in keeping both a user's interactive python shell happy, and github's CI/CD pipeline happy, at the same time.

... python packaging, sigh...

we 1) ask github CI/CD to setup a python3.10 we 2) ask conda to create an env with a python, but the package is NOT installed. i tried "pip install .", but that didnt work... because it used the first python, i imagine. now i will try calling the explicit pip of the CONDA python to install the package, before calling the test.

sometimes the environment is built with environemtn.yml, which has a slightly diff array of depencdencies in the yaml file, when compared to the conda-build target, meta.yaml./

… from environment.yaml

ilaflott

i reverted back to the state it was in when i introduced a tiny tweak. pipeline passes. will look at this in other issues/branches/PRs as you suggested- nontrivial for some reason.

honestly, i think we're fighting the pipeline+it's env much more than we are the packaging. those two sets of problems LOOK very similar, though.

ceblanton

Yes! thanks for this, @ilaflott

aradhakrishnanGFDL added 24 commits July 19, 2024 12:38

Create __init__.py

eefe4e2

rearranging dirs since the landscape has evolved

f6799f3

meta yml test updated

3f6c8a9

moving scripts

81a5757

Update meta.yaml

5bdc2f0

Update publish-conda.yml

9d6048f

Delete scripts/__init__.py

bdf6312

check for failure points without removing it all

Delete intakebuilder/__init__.py

00fc556

checking for failure points

Update publish-conda.yml

b8175a0

Update create-gfdl-catalog.yml

97283dd

Update publish-conda.yml

31e6e6e

Update meta.yaml

839b7bf

Delete scripts directory

6dd7410

Delete intakebuilder directory

6e8ae71

Update test_config.yaml

1fa7910

Update meta.yaml

6521d68

Update test_catalog.py

91752a9

Update test_ingestion.py

ef86c6d

Update test_import.py

78897ff

Update test_import.py

cf33250

Update meta.yaml

f13771b

Update test_import.py

dfa4629

Update test_import.py

d72f406

Update test_import.py

50944f3

aradhakrishnanGFDL marked this pull request as draft July 19, 2024 19:42

aradhakrishnanGFDL requested review from ilaflott and Ciheim July 19, 2024 19:42

aradhakrishnanGFDL added 3 commits July 19, 2024 15:46

Update create-gfdl-catalog.yml

8c87b1d

Update gen_intake_gfdl_runner.py

167f7b6

Update gen_intake_gfdl_notebook.ipynb

1b7dadf

aradhakrishnanGFDL mentioned this pull request Jul 19, 2024

Rename scripts to CatalogBuilder #4

Closed

aradhakrishnanGFDL marked this pull request as ready for review July 19, 2024 20:02

tweak meta.yaml, setup.py for requirements.

9c923fc

aradhakrishnanGFDL requested a review from ceblanton July 22, 2024 18:28

fixup import things. breakup some long lines.

f1ab693

ilaflott added 3 commits July 22, 2024 15:53

Update create-gfdl-catalog.yml

6965b94

`catalogbuilder.intakebuilder` not found in pipeline. b.c. the package isnt installed into the created environment. adding `pip install .` to check

... is it because github CI/CD doesn't preserve things between steps …

6b70e56

…like one always expects? .... wouldnt totally suprise me.

one more idea...

be073ac

ilaflott added 7 commits July 22, 2024 17:01

OK so there's a few things happening in the workflows.

49dc0af

sometimes the environment is built with environemtn.yml, which has a slightly diff array of depencdencies in the yaml file, when compared to the conda-build target, meta.yaml./

.... should prob activate the created conda env...

c12f60c

conda init before conda activate...

1435204

use miniconda3 docker image, auto-activate catalogbuilder env created…

a328d8c

… from environment.yaml

oops

59e72b1

revert to last working state before i broke it

a82f589

ilaflott approved these changes Jul 22, 2024

View reviewed changes

ceblanton approved these changes Jul 23, 2024

View reviewed changes

ilaflott merged commit 54f92f4 into main Jul 23, 2024
4 checks passed

ilaflott deleted the subpkgtest branch July 23, 2024 15:44

aradhakrishnanGFDL mentioned this pull request Jul 23, 2024

update docs #9

Open

ilaflott mentioned this pull request Jul 24, 2024

intakebuilder --> catalogbuilder NOAA-GFDL/fre-cli#128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changing packaging structure #5

Changing packaging structure #5

aradhakrishnanGFDL commented Jul 19, 2024

aradhakrishnanGFDL commented Jul 19, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

aradhakrishnanGFDL commented Jul 22, 2024

aradhakrishnanGFDL commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott left a comment

ceblanton left a comment

Changing packaging structure #5

Changing packaging structure #5

Conversation

aradhakrishnanGFDL commented Jul 19, 2024

aradhakrishnanGFDL commented Jul 19, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

aradhakrishnanGFDL commented Jul 22, 2024

aradhakrishnanGFDL commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott commented Jul 22, 2024

ilaflott left a comment

Choose a reason for hiding this comment

ceblanton left a comment

Choose a reason for hiding this comment