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

Fix converting of editable package #288

Merged
merged 16 commits into from
Aug 2, 2023
Merged

Fix converting of editable package #288

merged 16 commits into from
Aug 2, 2023

Conversation

Czaki
Copy link
Collaborator

@Czaki Czaki commented May 24, 2023

When I try to convert PartSeg to npe2 I found that there is a problem with the proper finding path to the module when the package is installed in editable mode.

Maybe it is somehow connected to changes to setuptools mentioned by @tlambert03 in the past.

Copy link
Collaborator Author

@Czaki Czaki May 24, 2023

Choose a reason for hiding this comment

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

I have added this file to simplify debugging my allow run by python -m npe2

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #288 (5315777) into main (d01b554) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #288   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         2813      2818    +5     
=========================================
+ Hits          2813      2818    +5     
Files Changed Coverage Δ
src/npe2/_inspection/_from_npe1.py 100.00% <100.00%> (ø)

for f_path in dist.files:
if "__editable__" in f_path.name:
path = Path(f_path.read_text().strip()) / top_module
break
Copy link
Member

Choose a reason for hiding this comment

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

@Czaki I have zero context here so take my review with a grain of salt, but if there are no tests here, could you at least add comments about why these lines are needed? eg "if the given npe1 plugin is installed in editable mode, then X doesn't work so we must do Y"

@tlambert03
Copy link
Collaborator

Yes @Czaki, this is the weird editable mode used by setuptools for a pyproject only package

@Czaki
Copy link
Collaborator Author

Czaki commented May 25, 2023

@tlambert03 partseg has setup.py (concat changelog with readme) , setup.cfg, and pyproject.toml files. So it looks like it is enough to contain pyproject.toml. And it looks like the problem surface on the drop of npe1 may be big without handling this.

How did you think? Should I implement some simple package to write a test for it, or just add comments?

@tlambert03
Copy link
Collaborator

Would be great to test it. I can't remember, did I make a test case for a setup.cfg repo here? If so, could you use it as a template to copy?

@Czaki
Copy link
Collaborator Author

Czaki commented Jun 2, 2023

@tlambert03 @jni I have finally created the proper test.

@Czaki Czaki requested a review from jni June 2, 2023 07:52
@nclack nclack merged commit 6913972 into napari:main Aug 2, 2023
18 checks passed
@Czaki Czaki deleted the fix_editable branch August 2, 2023 22:14
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.

4 participants