[setup.py] Fix Pypi install #29

Merged
tfardet merged 3 commits from pypi-install into main 10 months ago
Owner

Fix Pypi install by properly handling both flat and hierarchical file layouts.
This fixes #26

Fix Pypi install by properly handling both flat and hierarchical file layouts. This fixes #26
tfardet added the
bugfix
installation
labels 10 months ago
tfardet added 1 commit 10 months ago
tfardet requested review from gph82 10 months ago
Collaborator

Hi, I think you can get it even more compact, right?

# move important files to mpl_chord_diagram
move = (
    '__init__.py',
    '_info.py',
    'LICENSE',
    'chord_diagram.py',
    'gradient.py',
    'utilities.py',
    'example.py',
)
# Layout changes between git repo (flat) and Pypi (hierarchical) so check if
# flat layout files exist. If not, then all files are already in the subfolder

if not os.path.exists:
    dir_preexisits=False
    os.makedirs(directory)    
    for fname in move:
    	copy2(fname, directory)        
    
else:
    dir_preexisits=True

edit: forgot dir_preexisits
edit II: moved it up.

Hi, I think you can get it even more compact, right? ``` # move important files to mpl_chord_diagram move = ( '__init__.py', '_info.py', 'LICENSE', 'chord_diagram.py', 'gradient.py', 'utilities.py', 'example.py', ) # Layout changes between git repo (flat) and Pypi (hierarchical) so check if # flat layout files exist. If not, then all files are already in the subfolder if not os.path.exists: dir_preexisits=False os.makedirs(directory) for fname in move: copy2(fname, directory) else: dir_preexisits=True ``` edit: forgot dir_preexisits edit II: moved it up.
tfardet added 1 commit 10 months ago
Poster
Owner

How's that?

How's that?
Collaborator

sorry, I was away.

LGTM, but I'm having second thoughts on one thing. Since you're doing the rmtree after the setup(), it'd still be a good idea to use try/except, s.t. the rmtree gets run anyway, and an exception is raised anway. What do you think of something like

try:
   setup():
except:
   #something went wrong, we really don't care what
   if not dir_preexists:
      rmtree(directory, ignore_errors=True)
   raise

Otherwise failed installations will still leave a directory hanging there somewhere

sorry, I was away. LGTM, but I'm having second thoughts on one thing. Since you're doing the rmtree after the setup(), it'd still be a good idea to use try/except, s.t. the rmtree gets run anyway, and an exception is raised anway. What do you think of something like ``` try: setup(): except: #something went wrong, we really don't care what if not dir_preexists: rmtree(directory, ignore_errors=True) raise ``` Otherwise failed installations will still leave a directory hanging there somewhere
tfardet added 1 commit 10 months ago
Collaborator

thanks for changing my version to "finally", I was thinking of this since I woke up lol. Can't try it now, tho.

thanks for changing my version to "finally", I was thinking of this since I woke up lol. Can't try it now, tho.
Poster
Owner

OK, just approve the PR whenever you have checked that it works for you ;)

OK, just approve the PR whenever you have checked that it works for you ;)
Collaborator

can confirm

git clone https://codeberg.org/tfardet/mpl_chord_diagram.git
cd mpl_chord_diagram/
git fetch origin pypi-install:pypi-install
git checkout pypi-install
pip install .

works on clean conda environment, all deps downloaded, no subfolder mpl_chord_diagram left over after install. Also, python 3.8 can

>>> import mpl_chord_diagram

without any problems.

I'm approving 👍

can confirm ``` git clone https://codeberg.org/tfardet/mpl_chord_diagram.git cd mpl_chord_diagram/ git fetch origin pypi-install:pypi-install git checkout pypi-install pip install . ``` works on clean conda environment, all deps downloaded, no subfolder mpl_chord_diagram left over after install. Also, python 3.8 can ``` >>> import mpl_chord_diagram ``` without any problems. I'm approving 👍
gph82 refused to review 10 months ago
Poster
Owner

OK, cool, for can you use the "Approve" button in the "Files changed" tab, after clicking the "Review" button?
That way it is easily visible to everyone that the PR was reviewed and approved.

OK, cool, for can you use the "Approve" button in the "Files changed" tab, after clicking the "Review" button? That way it is easily visible to everyone that the PR was reviewed and approved.
Collaborator

Not seeing how to approve in the GUI anywhere 🤔

Not seeing how to approve in the GUI anywhere 🤔
tfardet requested review from gph82 10 months ago
gph82 approved these changes 10 months ago
dir_preexists = True
if e.errno != errno.EEXIST:
raise
Collaborator

why not:

if not os.path.exists(directory):
   os.makedirs(directory)
   dir_preexisits=False
else:
   dir_preexisits=True

edit: used spaces instead of Tabs, formatting got screwed up a bit

why not: ``` if not os.path.exists(directory): os.makedirs(directory) dir_preexisits=False else: dir_preexisits=True ``` edit: used spaces instead of Tabs, formatting got screwed up a bit
Poster
Owner

To have the proper scope (dir_preexisits is defined on the main scope)

To have the proper scope (``dir_preexisits`` is defined on the main scope)
tfardet marked this conversation as resolved
tfardet referenced this issue from a commit 10 months ago
tfardet merged commit 5c9c919dd6 into main 10 months ago
Poster
Owner

It works! 🥳
Thanks for the help and all the hard work!

It works! 🥳 Thanks for the help and all the hard work!
tfardet removed review request for gph82 10 months ago
tfardet deleted branch pypi-install 10 months ago
Collaborator

awesome!

I'm happy to help (I always learn something) but I'm also doing out of self-interest. As I told you at some point (i think 🤔 ?) I want to make my package depend on yours.

Before I even start to merge and test and publish, I want to make sure yours installable with pip. I'll ping you when the example case is up in the html docs. I want to use your package to coarse-grain this:
http://proteinformatics.uni-leipzig.de/mdciao/_images/notebooks_Comparing_CGs_Flares_20_1.png

awesome! I'm happy to help (I always learn something) but I'm also doing out of self-interest. As I told you at some point (i think 🤔 ?) I want to make my package depend on yours. Before I even start to merge and test and publish, I want to make sure yours installable with pip. I'll ping you when the example case is up in the html docs. I want to use your package to coarse-grain this: http://proteinformatics.uni-leipzig.de/mdciao/_images/notebooks_Comparing_CGs_Flares_20_1.png

Reviewers

gph82 approved these changes 10 months ago
The pull request has been merged as 5c9c919dd6.
Sign in to join this conversation.
Loading…
There is no content yet.