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: use widget from pymmcore-widgets main branch #269

Merged
merged 9 commits into from
Jul 28, 2023

Conversation

fdrgsp
Copy link
Contributor

@fdrgsp fdrgsp commented Jul 27, 2023

@ianhi linked to #264 (comment)
This PR updates napari-micromanager using the pymmcore-widgets in main branch.

@fdrgsp fdrgsp changed the title fix: use widget in pymmcore-widgets main branch fix: use widget from pymmcore-widgets main branch Jul 27, 2023
@ianhi
Copy link
Contributor

ianhi commented Jul 27, 2023

This raises the question of the dependency structure of the testing between these two repos. Would it maybe make more sense to do the following?

  1. In regular CI use the main branch of pymmcore-widgets/pymmcore-plus
  2. In testing prior to a release use the currently released versions

It seems that that would allow smooth development, but still make sure released versions are compatible.
@tlambert03 @fdrgsp

@tlambert03
Copy link
Member

the structure I prefer is for upstream libraries to test against the main branch of their dependents with -W ignore

  • useq tests main branch of pymmcore-plus
  • pymmcore-plus tests main branch of widgets
  • widgets tests main branch of napari-micromanager

While the downstream libraries test against the current release (NOT against the main branch).
Once the tests are in place, it's the upstream libraries responsibility to emit warnings rather than breakages, and let the downstream libraries test fail with a warning (rather than an actual error). This mimics the case that would happen with a downstream third party library we're not testing.

I know you've just jumped into an odd state, but we're actively working on this and you just caught us at a funny time.

self.position_groupbox.isChecked()
and len(self.position_groupbox.value()) > 0
not self.channel_widget.value()
or self.channel_widget._table.rowCount() == 1
Copy link
Member

@tlambert03 tlambert03 Jul 27, 2023

Choose a reason for hiding this comment

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

this is a usage of a private widget in pymmcore-widgets. If there is literally no public way to access this and you need it here, then you should make it public in pymmcore-widgets

(or make some sort of public method)

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

codecov bot commented Jul 28, 2023

Codecov Report

Patch coverage: 63.15% and project coverage change: -0.88% ⚠️

Comparison is base (433cb9e) 76.11% compared to head (6f4a186) 75.24%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #269      +/-   ##
==========================================
- Coverage   76.11%   75.24%   -0.88%     
==========================================
  Files          15       15              
  Lines         804      820      +16     
==========================================
+ Hits          612      617       +5     
- Misses        192      203      +11     
Files Changed Coverage Δ
src/napari_micromanager/_mda_handler.py 72.38% <ø> (-0.61%) ⬇️
src/napari_micromanager/_saving.py 55.00% <0.00%> (-0.94%) ⬇️
...rc/napari_micromanager/_gui_objects/_mda_widget.py 77.14% <62.06%> (-11.54%) ⬇️
...c/napari_micromanager/_gui_objects/_save_widget.py 90.62% <100.00%> (ø)
src/napari_micromanager/_gui_objects/_toolbar.py 90.38% <100.00%> (ø)
src/napari_micromanager/_util.py 66.66% <100.00%> (ø)
src/napari_micromanager/main_window.py 93.05% <100.00%> (+0.09%) ⬆️

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

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

Hooray, seeing some green lights here

@tlambert03 tlambert03 merged commit 60f13ca into pymmcore-plus:main Jul 28, 2023
36 of 46 checks passed
@fdrgsp fdrgsp deleted the fix_with_newest_wdgs branch May 27, 2024 18:53
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.

None yet

3 participants