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

moved startup script to own module #2547

Merged
merged 2 commits into from
Feb 23, 2022
Merged

Conversation

k-dominik
Copy link
Contributor

I forgot to include the startup script in the conda package for a reason ;)

I did try to use the setup.py scripts directive, but this will not work reliably as an entrypoint in conda.

note: this entrypoint has to run before any import of an ilastik module.

@emilmelnikov
Copy link
Member

Should we create an issue in the conda-build repo? This behaviour looks like a bug...

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #2547 (8312538) into main (28bb8dc) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2547      +/-   ##
==========================================
+ Coverage   51.56%   51.57%   +0.01%     
==========================================
  Files         514      514              
  Lines       60141    60141              
  Branches     9169     9169              
==========================================
+ Hits        31009    31019      +10     
+ Misses      27522    27514       -8     
+ Partials     1610     1608       -2     
Impacted Files Coverage Δ
lazyflow/operators/generic.py 88.97% <0.00%> (-0.20%) ⬇️
...ets/objectClassification/opObjectClassification.py 80.46% <0.00%> (-0.10%) ⬇️
lazyflow/slot.py 89.93% <0.00%> (+0.30%) ⬆️
lazyflow/operator.py 79.74% <0.00%> (+0.94%) ⬆️
lazyflow/operators/opPixelFeaturesPresmoothed.py 91.28% <0.00%> (+1.74%) ⬆️
lazyflow/operators/opReorderAxes.py 90.74% <0.00%> (+1.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28bb8dc...8312538. Read the comment docs.

@k-dominik
Copy link
Contributor Author

I think this might have been my bad...

@k-dominik
Copy link
Contributor Author

okay this one is really annoying. I tried different variants, also following the conda-build tests that test that it actually should be working, but I never got an exe file on windows for the script when supplied as scripts in setup.cfg (hence I closed #2549, and reopened this one.). I think this is related to conda/conda-build#3965.

@emilmelnikov
Copy link
Member

Do we even need an entrypoint for non-packaged ilastik variants? I think devs and people who want to use conda-based ilastik should be fine with python ilastik.py ... instead of ./ilastik ....

@k-dominik
Copy link
Contributor Author

I'm thinking not of devs but people who install ilastik using conda. They should be able to ilastik to start.

@k-dominik
Copy link
Contributor Author

turns out this might be some conda build issue (tracking it here: conda/conda-build#4385). So in order to keep things noarch, an explicit entrypoint is (currently) needed.

cc: @emilmelnikov

Copy link
Member

@emilmelnikov emilmelnikov left a comment

Choose a reason for hiding this comment

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

Maybe rename the module to ilastik.main, or at least ilastik.startup?

in order to include this in the package and be able to use it as a clean
entrypoint.
@k-dominik k-dominik merged commit 94cfa56 into ilastik:main Feb 23, 2022
k-dominik added a commit to ilastik/ilastik-conda-recipes that referenced this pull request Feb 23, 2022
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.

2 participants