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

Add the ability to run the script from anywhere #112

Merged
merged 16 commits into from
Feb 20, 2018

Conversation

DrewMonroe
Copy link
Member

  • Add the ability to run the script from anywhere
  • Add a command (youclidparser) that will run the parser straight from the commandline
  • Add symlinks to data files so they can be included with every installation of our repository

- Closes #111
- Added a MANIFEST.in file that will include our data files upon a
distribution
- Modify setup.py to include files required by MANIFEST.in
- Hopefully this will actually allow for us to distribute the package
and have someone use it
- Created a symbolic link that will allow us to run the script from the
command line (at least on Linux)
- The main parser will now output the absolute path to the HTML files to
allow the user to output their HTML anywhere
- Update the setup.py to require python 3.6
- Add an argument to the parser that will output the HTML that can be
used for a standalone distribution
- The symlink version of this file does not work if the default python3
on a user's system is not 3.6. This change will allow it to work, but
means that the command will take slightly longer to run. There's
probably a better way to do this
@saileshsimhadri
Copy link
Contributor

There should be a better way of finding the install directory in youclidparser

@@ -0,0 +1 @@
recursive-include youclidbackend/data/ *
Copy link
Member Author

@DrewMonroe DrewMonroe Feb 19, 2018

Choose a reason for hiding this comment

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

MANIFEST.in is used for sdist (source distribution). This will include the necessary data files. To be honest, I don't quite know if this file is required, since we have package_data in the setup.py file. If it's not, we can remove this at a later date.

@@ -3,6 +3,13 @@

setup(name='youclid',
version='0.1',
python_requires='>=3.6',
Copy link
Member Author

Choose a reason for hiding this comment

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

We use things that require python3.6, so we may as well ensure that the user has it when trying to install

'data/default.css',
'data/draw.js',
'data/index.js']},
include_package_data=True,
Copy link
Member Author

Choose a reason for hiding this comment

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

The package_data will ship our relevant files with the install. The key is the package where the data files live. Note that this means these files MUST exist inside our pacakge (hence the symlinks to the files in the frontend).

setup.py Outdated
'data/draw.js',
'data/index.js']},
include_package_data=True,
scripts=['bin/youclidparser']
Copy link
Member Author

Choose a reason for hiding this comment

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

This will add our youclidparser command to $PATH, allowing the user to run it

@@ -0,0 +1,4 @@
#!/usr/bin/sh
Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't the cleanest, but it means that no matter what version of python the user has as their default python3, python3.6 will be run. There's probably a better way to do this where we could just have this file symlink to main_parser.py (maybe #!python3.6 at the top of main_parser.py will force python3.6 to run?)

@@ -5,6 +5,14 @@
### Dependencies
- python3.6

### Installation
```bash
pip3 install git+git://github.com/YouClid/youclid.git@master#egg=youclid
Copy link
Member Author

Choose a reason for hiding this comment

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

If you try this now, it won't work, since this PR isn't merged into master yet. What you'd want is

pip3 install git+git://github.com/YouClid/youclid.git@feature/script-from-anywhere#egg=youclid

@josephsweeney
Copy link
Member

I think we also want to be able to just give a directory full of yc files and it just parses all of them. Or maybe you could do something like youclidparser *.yc -o *.html? Thoughts?

@josephsweeney
Copy link
Member

Also a small nitpick, is there a reason you went with youclidparser instead of just youclid?

The former seems more verbose and unless we're going to have something different for youclid there's not much of a downside.

@DrewMonroe
Copy link
Member Author

Fair point about the name, I updated this with 52ee1de. In regards to the directory of files: I agree that would be a good feature, probably not something that should be in this branch though. I've made an issue (#115)

@josephsweeney
Copy link
Member

@DrewMonroe we probably need to update the readme too right?

@DrewMonroe
Copy link
Member Author

Yeah, you're right, good call. I can do that right now.

Copy link
Member

@josephsweeney josephsweeney left a comment

Choose a reason for hiding this comment

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

It's looking good and pretty easy to do.

@josephsweeney josephsweeney merged commit caa513b into master Feb 20, 2018
@DrewMonroe DrewMonroe deleted the feature/script-from-anywhere branch February 20, 2018 14:54
DrewMonroe added a commit that referenced this pull request Feb 20, 2018
- This will fix issues discussed in #112
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