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

Remove GitLFS Dependency #40

Open
wants to merge 66 commits into
base: main
Choose a base branch
from
Open

Remove GitLFS Dependency #40

wants to merge 66 commits into from

Conversation

crvernon
Copy link
Member

@crvernon crvernon commented Mar 5, 2019

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:

  • Zipped the existing example directory and minted a DOI for it using Zenodo that establishes a supplemental data package for the Xanthos >= 2.2.0 releases,
  • Built an InstallSupplement class that fetches and unzips the Zenodo example data package into the Xanthos root directory,
  • Call the InstallSupplement class after setup has completed in setup.py,
  • Removed and replaced any GitLFS archived files that existed within the project,
  • Adjusted the paths in the test configuration files to recognize the working directory.

All tests have passed as executed via Travis.

Resolves #38

@crvernon
Copy link
Member Author

crvernon commented Jul 17, 2019

Alright @rplzzz we are ready to roll with a review on this one.

  • I made the reference data part of the package which is called accordingly
  • replaced all __file__ calls with proper package lingo using pkg_resources
  • dumped the auto-install of the example data on setup
  • removed all GitLFS pointers and objects (see Simple steps to uninstall Git LFS from your repository git-lfs/git-lfs#3026)
  • set up Travis-CI to install the example data for tests in the travis config outside
  • fixed all paths in all tests, etc.

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.

Copy link
Contributor

@rplzzz rplzzz left a 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.
Copy link
Contributor

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:
Copy link
Contributor

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]
Copy link
Contributor

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
Copy link
Contributor

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?

@crvernon crvernon requested a review from FeralFlows May 11, 2020 14:54
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.

Improvement: Remove GitLFS Dependency
2 participants