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

Run test on step 1 #41

Merged
merged 21 commits into from
Nov 28, 2023
Merged

Run test on step 1 #41

merged 21 commits into from
Nov 28, 2023

Conversation

kmilo9999
Copy link
Collaborator

  • Workflow to test step 1 integration

added workflow to test B01_SL_load_single_file
@kmilo9999 kmilo9999 requested a review from hollandjg November 15, 2023 16:41
Copy link
Member

@hollandjg hollandjg left a comment

Choose a reason for hiding this comment

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

Hi Camilo,
it looks like this isn't passing, because it's missing the `config.json file. You'll need to update the pyproject.toml line 154:

[tool.setuptools]
# If there are data files included in your packages that need to be
# installed, specify them here.
# TODO: ADD ANY DATA FILES WE WANT TO HAVE
package-data = {"icesat2_tracks" = ["config/*.json"]}

Copy link
Member

@hollandjg hollandjg 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 me!

Copy link
Member

@cpaniaguam cpaniaguam left a comment

Choose a reason for hiding this comment

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

Added a couple of clarifying questions and comments (that are most likely to be useful later?).

strategy:
fail-fast: false
matrix:
version: ['3.9']
Copy link
Member

Choose a reason for hiding this comment

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

Can other versions be added here? I think some code needed python 3.10 or 3.11 but I could be wrong on this.

runs-on: ubuntu-latest
steps:
- name: install mpi
run: sudo apt update && sudo apt-get install openmpi-bin openmpi-doc libopenmpi-dev
Copy link
Member

Choose a reason for hiding this comment

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

Why was this needed? Maybe add a comment?

- name: install icesat2-tracks using pip
run: pip install .
- name: first step B01_SL_load_single_file
run: python src/icesat2_tracks/analysis_db/B01_SL_load_single_file.py 20190502052058_05180312_005_01 SH_testSLsinglefile2 True
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: I guess this might be a command to add to CLI.

Copy link
Member

Choose a reason for hiding this comment

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

What is this file for? Maybe some comments can be added at the top to describe its purpose?

Copy link
Member

Choose a reason for hiding this comment

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

Is this file needed to set up the CI?

B=dict()
for i in A[0]['swatches']:
B[i['name']] = i['data']['values']
print(i['name'] + ' ' + str(i['data']['values']))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
print(i['name'] + ' ' + str(i['data']['values']))

I guess we are not refactoring stuff yet, but this print statement can probably be deleted and use a dict comprehension to populate B.

Comment on lines +13 to +57
rels=dict()


rels['plus']=B['red']
rels['minus']=B['blue']

rels['blue']=B['blue']
rels['lightblue']=B['cascade3']
rels['darkblue']=B['cascade1']


rels['white']=B['white']
rels['gridcolor']=B['gridcolor']
rels['grey']=B['gray']

rels['orange']=B['orange']
rels['red']=B['red']
rels['green']=B['green']

rels['cascade1']=B['cascade1']
rels['cascade2']=B['cascade2']
rels['cascade3']=B['cascade3']
rels['cascade4']=B['gridcolor']

rels['rascade1']=B['rascade2']
rels['rascade2']=B['rascade1']
rels['rascade3']=B['rascade3']

rels['aug1']=B['orange']
rels['aug2']=B['green']

rels['gt1l']=B['rascade1']
rels['gt1r']=B['rascade3']

rels['gt2l']=B['green1']
rels['gt2r']=B['green2']

rels['gt3l']=B['cascade1']
rels['gt3r']=B['blue']

rels['group1']=B['rascade1']
rels['group2']=B['green1']
rels['group3']=B['cascade1']

B['rels']=rels
Copy link
Member

Choose a reason for hiding this comment

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

This can probably be refactored as well but I guess we'll get that later.

Copy link
Member

Choose a reason for hiding this comment

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

Can the template generic comments be removed now that the project is (completely?) set up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

referenced in #43

@kmilo9999 kmilo9999 merged commit c4f7b01 into main Nov 28, 2023
1 check passed
@kmilo9999 kmilo9999 deleted the feature-test-step1 branch November 28, 2023 21:44
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