search

pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.21k stars 356 forks source link

Remove Array2D from Pharo core #12140

Closed Ducasse closed 15 hours ago

Ducasse commented 1 year ago

We should remove Array2d from Pharo core and point people to http://github.com/pharo-containers In addition Array2D is not a matrix so we should remove the math part (that is partially implemented and probably in a bad way). For Matrix people should use Polymath implementation.

hernanmd commented 1 year ago

I found only one reference to Array2D in Pharo 11, this is Roassal3's method RSDSM>>createShapes But I see no users of RSDSM or its subclass RSDSMStronglyConnected in the default image.

Maybe the BaselineOfRoassal2 could include Pharo-Containers and adapt the methods?

createShapes

...
matrix := CTArray2D width: objectsX size height: objectsY size.
...

@akevalion ?

Ducasse commented 1 year ago

Yes it would be nice in fact :)

akevalion commented 1 year ago

Hi, currently Roassal is using Array2D. CTArray2D is not present in pharo12, should I remove then the dependency to Array2D?

akevalion commented 1 year ago

I have removed dependency of this class in roassal.

jordanmontt commented 1 year ago

Hello, we should benchmark the two implementations of Array2D before removing it. Also, it makes sense to have a 2D array by default in Pharo. Because now with the one of pharo-contributions people need to load it by hand...

Ducasse commented 1 year ago

Seb I'm sorry but we should not load all the collections by default. People should load what they need.

akevalion commented 1 year ago

This pull request https://github.com/pharo-project/pharo/pull/15148 has changed Array2D as deprecated class. We can remove it, but people may complain about it

lugu commented 3 days ago

Issue referenced at https://github.com/pharo-graphics/Bloc/issues/643

jordanmontt commented 15 hours ago

Closing this because Array2D is already removed from the default Pharo 13 image