-
Notifications
You must be signed in to change notification settings - Fork 16
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
Remove GitLFS Dependency #40
base: main
Are you sure you want to change the base?
Conversation
Alright @rplzzz we are ready to roll with a review on this one.
The protocol for installing the example data for Xanthos is now conducted by executing the following after install: from xanthos import InstallSupplement
InstallSupplement("<path to the directory you wish to store the example data in>") Once the the current checks have completed - they will pass - you can review. After I get the OK from you, I will add this install protocol to the README (BTW, we need a readthedocs.io page) and mint the new example data for usage with the most recent version. |
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.
Everything looks good. Can you resolve the conflicts with master? It looks like they're all binary files.
@@ -16,8 +16,7 @@ Hejazi, Mohamad I <[email protected]> | |||
|
|||
|
|||
# Notice | |||
Xanthos currently supports both Python 2.7 and Python 3.3+, however future support for Python 2 is not guaranteed. This repository also uses the Git Large File Storage (LFS) extension (see https://git-lfs.github.com/ for details). Please run the following command before cloning if you do not already have Git LFS installed: | |||
`git lfs install`. | |||
Xanthos currently supports both Python 2.7 and Python 3.3+, however future support for Python 2 is not guaranteed. This repository no longer uses the Git Large File Storage (LFS) extension. Instead, we now have supplementary data to Xanthos archived on Zenodo. This data is automatically downloaded from our current release upon running installing Xanthos locally. |
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.
It's not exactly automatic, right? My understanding is that you have to run the install command. Is this the readme update you were talking about in your last comment?
# get Python major version number | ||
pyversion = sys.version_info.major | ||
|
||
if pyversion <= 2: |
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.
The <=
is a nice touch. Good luck, python 1.x people.
current_version = get_distribution('xanthos').version | ||
|
||
try: | ||
data_link = InstallSupplement.DATA_VERSION_URLS[current_version] |
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.
Nicely done. I hadn't thought of tying the data version to the model version, but it makes sense.
args = parser.parse_args() | ||
|
||
zen = InstallSupplement(args.example_data_directory) | ||
del zen |
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 del
statement needed? The object should be gc'd when the script finishes anyhow, right? Or is there something I'm missing?
The GitLFS dependency for this repository has proven to be problematic for many users. However, Xanthos still has the need to include larger files as an example directory. To solve this issue I completed the following:
InstallSupplement
class that fetches and unzips the Zenodo example data package into the Xanthos root directory,InstallSupplement
class aftersetup
has completed insetup.py
,test
configuration files to recognize the working directory.All tests have passed as executed via Travis.
Resolves #38