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

rectprops update #66

Closed
wants to merge 1 commit into from
Closed

rectprops update #66

wants to merge 1 commit into from

Conversation

afni-dglen
Copy link
Contributor

Re-forked and new pull request to implement changes for deprecated rectprops with props in matplotlib function call. This version checks for older versions of matplotlib to continue with old behavior.

@rgbayrak rgbayrak self-requested a review April 25, 2024 14:50
@rgbayrak
Copy link

@smoia do you know what is this check that failed? .pre-commit-config.yaml is not a file

@smoia
Copy link
Member

smoia commented Jul 9, 2024

Hey @afni-dglen ! Could you rebase this PR on the latest master, please? This should take care of a few issues. After that, @maestroque , do you want to give a try at a PR review? Otherwise @me-pic and @m-miedema could help with it?

@maestroque maestroque self-requested a review July 16, 2024 11:28
Copy link
Contributor

@maestroque maestroque left a comment

Choose a reason for hiding this comment

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

If we are going to allow all matplotlib versions, we should remove the dependency constraint in setup.cfg

@maestroque
Copy link
Contributor

Hey @afni-dglen !
I believe the things that remain for this PR are:

  • rebasing from master branch (this will also take care of pre-commit because it was added in a later patch)
  • removing the matplotlib constraint from setup.cfg

This PR is currently blocking physiopy/phys2denoise#54, due to the incompatibility of matplotlib throughout peakdet, phys2denoise and physutils, so it would be really appreciated if this could be handled soon.

Btw @smoia I think once this is merged we should do a new release in peakdet, since the current release is already outdated, and clones dependencies such as gooey and wxpython.

@maestroque
Copy link
Contributor

Also, another question is that could we just change the version here at >=3.9, as in phys2denoise and physutils and settle with props instead?
Is there any other compatibility issue with other packages I'm missing?

@afni-dglen afni-dglen mentioned this pull request Jul 18, 2024
18 tasks
@afni-dglen
Copy link
Contributor Author

I think I took care of both those issues. Rebasing was problematic, so another fork and a new pull request.

#74

I'll close this one then.

@afni-dglen afni-dglen closed this Jul 18, 2024
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