-
Notifications
You must be signed in to change notification settings - Fork 1
Implement publish product feature #1
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
Implement publish product feature #1
Conversation
forman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall!
- See all my suggestion and comments
- Copyright (c) 2024 --> 2025
- In CLI, configure logger to print nicely to stdout
README.md
Outdated
| execute the steps below: | ||
|
|
||
| ```commandline | ||
| conda env create -f environment.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| conda env create -f environment.yml | |
| conda env create |
| f"Last error: {last_exception}" | ||
| ) | ||
|
|
||
| def _get_spatial_extent(self) -> SpatialExtent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor function body: code triplication
| # For irregular gridding | ||
| x_min, x_max = (float(self.dataset.x.min()), float(self.dataset.x.max())) | ||
| y_min, y_max = (float(self.dataset.y.min()), float(self.dataset.y.max())) | ||
| return SpatialExtent([[x_min, y_min, x_max, y_max]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ok? Transform to geographic using grid mapping?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it works for irregular gridding with y,x coordinates. Tested it with polar dataset to confirm. For a pystac collection, I just need the bbox that encompasses the data. Any other considerations here ?
deep_code/cli/publish.py
Outdated
|
|
||
|
|
||
| @click.command(name="publish-dataset") | ||
| @click.option( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer, you would not force users putting credentials into a text file.
If it is really required for some reason, make it a hidden file with known name that is picked up from the root of the repo and without extension. So:
- Remove CLI option.
- Name the file
.gitaccess. A template could be created bydeep-code new .... - Provide env-var alternative:
GH_TOKEN,GH_USER. Let the tool document this.
Co-authored-by: Norman Fomferra <norman.fomferra@brockmann-consult.de>
Co-authored-by: Norman Fomferra <norman.fomferra@brockmann-consult.de>
Implemented CLI and python API that enables generating OSC collection from a DeepESDL dataset and publishing it to open science catalog as a static file in open-science-metadata repository.