Euphoria
Ticket #260:
Review sets.e
-
Reported by
mattlewis
Oct 29, 2010
There are many routines that don't make much sense, are difficult to understand, and not commented at all. As of writing this ticket, sets.e is the most untested file.
This file will be a nightmare to maintain. We find comments like the following:
-- Description: Wrapped by embed_union so as to avoid some type checks.
Many (most?) routines are thin wrappers around the actual routines that to the work, such as union() -> union1().
Other routines, like the "fiber" related routines, are difficult to figure out, and probably of limited utility.
The map() type, while potentially useful, uses a very questionable data structure (mixing actual element data and metadata in the same sequence), and has horrible documentation.
Details
1. Comment by jeremy
Oct 29, 2010
I'm for keeping a minimal set of routines that make the file functional. If this is not easily accomplished, I say remove it entirely and when we have time to maintain it, grab it from SVN and being work on it for a post 4.0.0 release.
I don't think this file AS-IS should be included in 4.0.0, but I don't think that 4.0.0 should be held up because this file is so bad. So... what I'm saying is, if someone wants, make a minimally useful sets.e that is sane or remove sets.e and close this ticket.
2. Comment by jeremy
Oct 29, 2010
Oh... some rationale... sets.e is not essential to a language. Nice, but not essential.
3. Comment by mattlewis
Oct 29, 2010
In justification of the "Blocking" severity: I don't think we should release stuff that no one really understands or can justify. If someone can justify bits of the file, and can improve the documentation so that people other than the original author can figure them out and use the routines, then I'm fine with keeping.
Here is my rough, initial TODO list:
- Keep the "Inclusion and Belonging" and "Basic set-theoretic operations" and anything those depend on.
- Get rid of everything else. Most of this goes beyond simple sets, and assumes ordering and additional stuff that would probably be better off in a separate, more specialized file. However, beyond the basic sets stuff, I don't think we need this, unless we can prove demand, and even then, there is a lot of work needed in this file.
4. Comment by DerekParnell
Oct 29, 2010
I had begun working on sets.e and have gotten a fair way into it. However, it is really a confused module. It should be split into two parts really - one dealing with sets and one dealing with matrices. The docs and comments are terrible as they assume a high level of formal maths background.
5. Comment by DerekParnell
Oct 31, 2010
Can we defer resolution until after 4.0?
6. Comment by jimcbrown
Oct 31, 2010
I'm also in favor of pulling sets.e out for 4.0 release, and defering the rewrite of it to 4.1.0
7. Comment by mattlewis
Oct 31, 2010
I agree with Jeremy. We should simply pull the whole thing rather than leave it in and plan to remove stuff later.
8. Comment by jimcbrown
Nov 01, 2010
Removed from 4.0.0 release, and changing the milestone to 4.1.0
9. Comment by SDPringle
Nov 02, 2010
removing sets.e from manual.af
10. Comment by ne1uno
Nov 09, 2010
removed from euphoria/stddebug.e
11. Comment by SDPringle
Nov 21, 2010
The kind of maps supported in sets from some n and m from {1,2,..,n} into {1,2,..,m}.
Then you use two sets s1 and s2 and an index map im to get the functionality of general maps: The map's co-domain must be the same size as s1 and the domain the same size as s2. If all these are true then we can say: for any k, with 1 <= k <= length(s1)-2, s1[k] maps to s2[im[k]]. It looks like cardinal(s) should be length(s)-2, rather than length(s).
To me, there should be three files:
- sets.e : Which are sets of arbitrary EUPHORIA objects
- index_map.e : Which are for maps that allow you to map {1,2,...,n} into {1,2,...,m} for some natural n, and m.
- map.e : General maps that include two set objects and an index_map in its structure.
12. Comment by jimcbrown
Nov 21, 2010
I thought this ticket was assigned to Derek or Matt. Did someone clear the assigned to list on purpose?
Problem with your proposal is that we already have a map.e that is a dictionary (made of a set and a sequence that does not require an index_map - but could be generalized to two sets and an index_map, with the index_map helping to handle cases where two keys return the same value).
I guess we could go with sets.e / index_map.e / general_map.e / map.e (or maybe sets.e / index_map.e / math_map.e / map.e ) or sets.e / index_map.e / map.e / dict.e
I hate the names index_map.e, general_map.e, math_map.e but that's a rather minor issue.
13. Comment by jeremy
Nov 28, 2010
Moving to a normal status, reviewing this is not a blocking severity. i.e. it does not block someone from progressing with their application.
14. Comment by SDPringle
Nov 28, 2010
I have looked into this quite a bit and though index_map.e is also dictionaries. They are only dictionaries from sequence index to sequence index. sets.e are sequences with order and no repeats. index_maps are immutable. There are no support routines for adding values to index_maps.
- sets.e
- index_map.e
- map.e (already existing and leave it alone)
In std/convert.e we can add routines to decompose a map into an index_map, and two sets and another to do the reverse.
15. Comment by DerekParnell
Nov 28, 2010
I want to get away from the term map altogether. This module currently deals with mathematical constructs...
- Sets (eg. union, intersect, disjoint, mapping set A to set B, etc ...)
- Matrices
It really has nothing to do with dictionaries (i.e. associative arrays/hash maps), and the matrix stuff has a lot missing from its implementation.
16. Comment by SDPringle
Nov 29, 2010
Matrices? How do you multiply { {2,2}, {3,3} } and { {4,5},{3,2} } using this library?
17. Comment by SDPringle
Jan 18, 2013
I will borrow Derek's form of speaking and say that the flying capabilities of my bicycle has a lot missing in its implementation.
18. Comment by jimcbrown
Jan 21, 2013
sets.e appears to be missing in my copy of trunk. So... over 2 years, and it hasn't been reintroduced yet. Perhaps we can close this ticket out saying that sets.e was completely pulled altogehter?
19. Comment by SDPringle
Feb 18, 2013
I agree with JIM it is time we close this ticket.
GitHub