compas icon indicating copy to clipboard operation
compas copied to clipboard

New small utility functions

Open romanarust opened this issue 2 years ago • 3 comments

What type of change is this?

  • [ ] Bug fix in a backwards-compatible manner.
  • [x] New feature in a backwards-compatible manner.
  • [ ] Breaking change: bug fix or new feature that involve incompatible API changes.
  • [ ] Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • [x] I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • [x] I ran all tests on my computer and it's all green (i.e. invoke test).
  • [x] I ran lint on my computer and there are no errors (i.e. invoke lint).
  • [x] I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • [x] I have added necessary documentation (if appropriate)

romanarust avatar Jul 28 '21 17:07 romanarust

ok, i've implemented changes and added tests.

romanarust avatar Jul 31 '21 09:07 romanarust

@romanarust is there any reason not to merge this?

beverlylytle avatar Aug 10 '21 07:08 beverlylytle

We discussed this today in the compas dev meeting. flatten as it currently is written is used quite a lot in compas code and in particular changing it to what I suggested would flatten lists of lists of points to just a list of floats rather than the expected list of points, breaking many things. It was suggested that this new flatten be made an internal function, say _flatten or so, for use within reshape and that before the release of version 2.0 current usage of flatten would be switched to itertools.chain and the new _flatten would be made external. Does this make sense?

beverlylytle avatar Aug 30 '21 12:08 beverlylytle

Ping here again! remove the changes to the "flatten" function, think it is ready now

romanarust avatar Jun 19 '23 12:06 romanarust

@romanarust black is complaining

tomvanmele avatar Jun 19 '23 12:06 tomvanmele