Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Add parentheses to make tuple explicit. Bump patch version #12

Closed
wants to merge 1 commit into from

Conversation

lillekemiker
Copy link
Contributor

The original lack of parentheses raises syntax errors in Python 3

@paulmelnikow
Copy link
Member

Hey there! 😀

I'd forked this at https://github.com/metabolize/blmath and would like to merge those changes back into this repo. I can probably do that within the next few days.

This change is already made in the fork:

https://github.com/metabolize/blmath/blob/8ea8d7be60349a60ffeb08a3e34fca20ef9eb0da/blmath/geometry/primitives/plane.py#L472

Meanwhile if you'd like to test the new version you can install metablmath (which is importable as blmath).

@paulmelnikow
Copy link
Member

Also, worth mentioning: in the long-term, since this project was always a bit of a grab bag (as the name implies!) the plan is to deprecate it and replace it with smaller, more focused libraries.

@algrs
Copy link
Contributor

algrs commented Sep 9, 2019

Hi Paul! Would a good plan here be to merge metablmath back upstream and then do some cleanup from there? I was just cleaning up and getting everything to build in python 3 (including the cholmod stuff, which it looks like hasn't come over to python3 in metablmath), but it'd be great to start from the newest version. If you wanted to do the upstream merge, I'd be happy to apply the cleanup I've been working on. Then we can start having blmath just delegate to things like vg with deprecation warnings saying to switch over.

@paulmelnikow
Copy link
Member

Hi Paul!

👋 👋

Would a good plan here be to merge metablmath back upstream and then do some cleanup from there? I was just cleaning up and getting everything to build in python 3 (including the cholmod stuff, which it looks like hasn't come over to python3 in metablmath), but it'd be great to start from the newest version. If you wanted to do the upstream merge, I'd be happy to apply the cleanup I've been working on.

That all sounds good. I'll take a few minutes to do the merge and make a new release so you can get the ball rolling. (P.S. Excited to have this ball rolling! 🎉)

Then we can start having blmath just delegate to things like vg with deprecation warnings saying to switch over.

That's definitely an option.

Another alternative is to think of blmath as a legacy dependency. We could make minimal changes needed here to keep things working, like patching cholmod for python3. Though the main push would be to build up the new, clean versions in vg, polliwog, and other targets, and transition dependent code to use those dependencies directly.

It feels like replacing code in blmath with shims and verifying compatibility by bumping deps in dependent code could involve a fair amount of work and represents further investment in this library, which really should be wound down. And there would still be a need at a later date to remove the compatibility layer, and have the dependent code updated to call the new libraries directly.

That said, if delegating with a deprecation warning is the best way forward for yous, and you want to do the lifting, I'd be fine signing off on it.

paulmelnikow added a commit that referenced this pull request Sep 9, 2019
@algrs
Copy link
Contributor

algrs commented Sep 9, 2019

That's fair -- maybe the best thing is deprecation warnings on import for things that have been broken out and no shim layer at all. Mostly I just want to start gently pushing people in the direction of deprecation.

(I'm also excited to be moving on this, after more than a year of talking about it!)

@paulmelnikow
Copy link
Member

That's fair -- maybe the best thing is deprecation warnings on import for things that have been broken out and no shim layer at all.

That sounds great to me.

(I'm also excited to be moving on this, after more than a year of talking about it!)

❤️

@paulmelnikow
Copy link
Member

Opened #15 and #16 to track that. 🙌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants