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

feat: Allow passing a config from command line #257

Merged
merged 6 commits into from
Feb 2, 2024

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Feb 24, 2023

Allows for

napari-micromanager --config path/to/config.cfg

closes #252

@ianhi ianhi linked an issue Feb 24, 2023 that may be closed by this pull request
@tlambert03 tlambert03 changed the title ENH: Allow passing a config from command line feat: Allow passing a config from command line Jul 10, 2023
@tlambert03
Copy link
Member

happy to get this in @ianhi ... but there are two comments that need addressing and merge conflicts to fix

@ianhi
Copy link
Contributor Author

ianhi commented Jul 26, 2023

but there are two comments that need addressing and merge conflicts to fix

Yup, I'm catching up on some github things this week. hope to address this and several other things soon.

@ianhi ianhi closed this Jul 28, 2023
@ianhi ianhi reopened this Jul 28, 2023
@tlambert03
Copy link
Member

very strange. I can't reproduce these errors locally, even with new environment

@tlambert03 tlambert03 closed this Jul 28, 2023
@tlambert03 tlambert03 reopened this Jul 28, 2023
@tlambert03
Copy link
Member

tlambert03 commented Jul 28, 2023

oh actually, I can reproduce it if I run python -m pytest -v --color=yes --cov=napari_micromanager --cov-report=xml ... (like we do on CI). I think you need to touch up the sys.argv parsing (or modify the test)

import napari
from napari.qt import QtViewer

with patch("napari.run") as mock_run:
with patch("qtpy.QtWidgets.QMainWindow.show") as mock_show:
main()
with patch("napari.utils.notifications.show_warning") as mock_warning:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that this patch isn't actually patching (i.e. the warning still gets displayed) and it is never called.

Any idea what I'm doing wrong here?

Copy link
Member

Choose a reason for hiding this comment

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

not immediately, but you know what... just use warnings.warn(), napari will intercept that anyway and show a warning in the UI (or at least it used to?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm it doesn't seem to capture it in the same way. maybe i need to change the level

Copy link
Member

Choose a reason for hiding this comment

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

oh, it's possible that the napari warnings hook hasn't yet been invoked by this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually scratch that, now the show warnign is also not showing, so maybe just going away to quickly. it still shows up in teh terminal

@ianhi
Copy link
Contributor Author

ianhi commented Jul 28, 2023

oh actually, I can reproduce it if I run python -m pytest -v --color=yes --cov=napari_micromanager --cov-report=xml ... (like we do on CI). I think you need to touch up the sys.argv parsing (or modify the test)

This was very helpful thank you!

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (c33cb23) 78.24% compared to head (2375f5f) 79.04%.
Report is 2 commits behind head on main.

Files Patch % Lines
src/napari_micromanager/__main__.py 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #257      +/-   ##
==========================================
+ Coverage   78.24%   79.04%   +0.80%     
==========================================
  Files          15       15              
  Lines         671      692      +21     
==========================================
+ Hits          525      547      +22     
+ Misses        146      145       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tlambert03
Copy link
Member

i finished this up

@tlambert03 tlambert03 closed this Feb 2, 2024
@tlambert03 tlambert03 reopened this Feb 2, 2024
@tlambert03 tlambert03 enabled auto-merge (squash) February 2, 2024 18:40
@tlambert03 tlambert03 closed this Feb 2, 2024
auto-merge was automatically disabled February 2, 2024 19:59

Pull request was closed

@tlambert03 tlambert03 reopened this Feb 2, 2024
@tlambert03 tlambert03 merged commit 41a9943 into pymmcore-plus:main Feb 2, 2024
31 of 32 checks passed
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.

Allow taking config file as command line argument
2 participants