Skip to content
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

Split main into separate function and add entry point #83

Merged
merged 10 commits into from
Aug 21, 2024

Conversation

blimlim
Copy link
Collaborator

@blimlim blimlim commented Aug 21, 2024

This PR addresses the ESM1.5 conversion driver part of #81, splitting the main block into separate main() and parse_args() functions, which allows for an entry point to be added to the pytproject.toml file.

Adding entry points to um2netcdf will be addressed separately in #82.

I'm unsure how to test that the entry point is working, and so suggestions on that would be great too!

Copy link
Collaborator

@truth-quark truth-quark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only thing missing is an entry point CI test to "run" the conversion driver script in a simple fashion.

Do you want to try copying & modifying the old entry point test from the CI.yml here:
2c7b355, and see if it can run the conversion driver script with --help?

@aidanheerdegen
Copy link
Member

@blimlim I debugged the conda packaging by copying the commands from the CI to build and install the package locally.

So something like

conda build -c accessnri -c conda-forge -c coecms . --no-anaconda-upload --output-folder=./build
conda env create -f conda/environment.yml 
conda activate um2nc 
conda install -c conda-forge -c accessnri -c coecms -c file://$(readlink -f build) um2nc

@aidanheerdegen
Copy link
Member

The problem is with package discovery

https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#custom-discovery

The package isn't discovered with the current settings because there isn't a __init__.py file in the umpost directory. So it isn't even installed correctly (see I said there would always be some problem!).

Just add one (it can be empty), e.g.

touch upmost/__init__.py

and it should all work as it is now.

(um2nc) $ esm1p5_convert_nc 
usage: esm1p5_convert_nc [-h] [--quiet] [--delete-ff] current_output_dir
esm1p5_convert_nc: error: the following arguments are required: current_output_dir
(um2nc) $ git status -s
?? build/
?? umpost/__init__.py
(um2nc) $ 

@aidanheerdegen
Copy link
Member

Doh. We need to also remove the accessnri channel when installing the um2nc package from the local directory, otherwise it will likely pick up the version that is already installed there rather the local one.

https://github.com/ACCESS-NRI/um2nc-standalone/pull/83/files#diff-3ab46ee209a127470fce3c2cf106b1a1dbadbb929a4b5b13656a4bc4ce19c0b8L86

or make the local file channel the first channel to ensure it picks that version up.

@aidanheerdegen
Copy link
Member

@blimlim
Copy link
Collaborator Author

blimlim commented Aug 21, 2024

I moved the local file to the first channel and it's working now!

@aidanheerdegen
Copy link
Member

I moved the local file to the first channel and it's working now!

Quick! Merge before anything changes! ;)

Copy link
Collaborator

@truth-quark truth-quark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to go!

@blimlim blimlim merged commit 3a3f4ca into develop Aug 21, 2024
4 checks passed
@blimlim blimlim deleted the 81/esm-driver-entry-point branch August 21, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants