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

Gloria downloader #104

Merged
merged 11 commits into from
Jun 8, 2023
Merged

Conversation

hazimhussein
Copy link
Collaborator

This branch contains the implementation of the GLORIA downloader and the related documentation

@konstantinstadler
Copy link
Member

seems like you did not format (black and isort) the pull request. You can have a look into the format_and_test.sh file on how to do that. This is a linux file, but you can copy paste the commands in cml or write your own bat file (actually, you could translate/make a format_and_test.bat similar to the sh file for convinience, just push it with a pull request when you see that it is working)

tests/test_util.py Show resolved Hide resolved
doc/source/notebooks/advanced_group_stressors.ipynb Outdated Show resolved Hide resolved
@konstantinstadler
Copy link
Member

konstantinstadler commented Apr 19, 2023 via email

@hazimhussein
Copy link
Collaborator Author

hazimhussein commented Apr 19, 2023 via email

@konstantinstadler konstantinstadler dismissed their stale review June 8, 2023 11:23

changes have been implemented as requested, all done

Copy link
Member

@konstantinstadler konstantinstadler left a comment

Choose a reason for hiding this comment

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

great work!

@@ -91,6 +94,11 @@
},
}

GLORIA_CONFIG = {"datafiles": {}}

with open(os.path.join(os.path.abspath(__ROOT), "../tools/gloria_urls.json"), "r") as f:
Copy link
Member

Choose a reason for hiding this comment

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

Please put this data file in the constants.py file, make a new entry CONFIG_FILE and just put it there

Copy link
Member

Choose a reason for hiding this comment

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

with that you can remove _ROOT then. The idea is that we dont have path specs in the code, but all gathered in constants

@konstantinstadler konstantinstadler merged commit 93633eb into IndEcol:master Jun 8, 2023
13 checks passed
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.

None yet

2 participants