renamed Is(Skeletal)FiniteSet(Map) -> IsObject/MorphismIn(Skeletal)CategoryOfFiniteSets

[mohamed-barakat]

No description provided.

[zickgraf]

Some things:

1. The old names were much shorter and thus potentially better suited for didactical purposes. (I'm not against the change, I just wanted to mention it).
2. We should keep FinSets and SkeletalFinSets consistent, i.e. also rename IsFiniteSet and IsFiniteSetMap.
3. You can simply comment the synonyms for Julia, they should not be needed there.

[mohamed-barakat]

Some things:

1. The old names were much shorter and thus potentially better suited for didactical purposes. (I'm not against the change, I just wanted to mention it).

I don't like these names for two reasons:

• there is nothing called skeletal finite set. The word skeletal is referring to the category.
• I don't like the convention of having the suffix Object/Morphism, especially with the name of the category is too long. I prefer IsObject/MorphismIn*, where * can be an arbitrary long name.

2. We should keep FinSets and SkeletalFinSets consistent, i.e. also rename IsFiniteSet and IsFiniteSetMap.

Done.

3. You can simply comment the synonyms for Julia, they should not be needed there.

Done.

Thanks.

[zickgraf]

The GAP to Julia conversion now has support for DeclareSynonym: homalg-project/PackageJanitor@d4fc09e This was much easier then I expected, I thought GAP would handle synonyms in a much more elaborate way than just binding a global variable.

Also, as mentioned verbally, I suggest renaming IsObjectInCategoryOfFinSets to IsObjectInFinSets or IsObjectInCategoryOfFiniteSets, i.e. either use the shorthand FinSets or the full description CategoryOfFiniteSets (of course including similar renames for morphisms and SkeletalFinSets).

[mohamed-barakat]

The GAP to Julia conversion now has support for DeclareSynonym: homalg-project/PackageJanitor@d4fc09e This was much easier then I expected, I thought GAP would handle synonyms in a much more elaborate way than just binding a global variable.

Nice. Still, the CI is failing.

Also, as mentioned verbally, I suggest renaming IsObjectInCategoryOfFinSets to IsObjectInFinSets or IsObjectInCategoryOfFiniteSets, i.e. either use the shorthand FinSets or the full description CategoryOfFiniteSets (of course including similar renames for morphisms and SkeletalFinSets).

Done.

[zickgraf]

The GAP to Julia conversion now has support for DeclareSynonym: homalg-project/PackageJanitor@d4fc09e This was much easier then I expected, I thought GAP would handle synonyms in a much more elaborate way than just binding a global variable.

Nice. Still, the CI is failing.

You have to remove the comments for Julia again.

[mohamed-barakat]

You have to remove the comments for Julia again.

My bad.

Please do not merge until the PR on CategoricalTowers passes. I will let you know.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (308ee8b) 100.00% compared to head (59271a5) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #210   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines         1882      1890    +8     
=========================================
+ Hits          1882      1890    +8     

Files Changed	Coverage Δ
PackageInfo.g	100.00% <100.00%> (ø)
gap/CompilerLogic.gi	100.00% <100.00%> (ø)
gap/FinSets.gd	100.00% <100.00%> (ø)
gap/FinSets.gi	100.00% <100.00%> (ø)
gap/SkeletalFinSets.gd	100.00% <100.00%> (ø)
gap/SkeletalFinSets.gi	100.00% <100.00%> (ø)
gap/init.gi	100.00% <100.00%> (ø)
...fFiniteSetsWithMorphismsGivenByListsPrecompiled.gi	100.00% <100.00%> (ø)
read.g	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mohamed-barakat]

Finally, thank you for your help :)

[mohamed-barakat]

The merge in CategoricalTowers is done. This can now be merged.

[zickgraf]

Please provide full backwards compatibility. As far as I can tell, the constructors are missing.

[mohamed-barakat]

Please provide full backwards compatibility. As far as I can tell, the constructors are missing.

Done.

[mohamed-barakat]

I would suggest dropping the backward compatibility for CategoryOfSkeletalFinSetsWithMorphismsGivenByListsPrecompiled since it is not used outside of FinSetsForCAP.

[zickgraf]

I would suggest dropping the backward compatibility for CategoryOfSkeletalFinSetsWithMorphismsGivenByListsPrecompiled since it is not used outside of FinSetsForCAP.

I agree, CategoryOfSkeletalFinSetsWithMorphismsGivenByListsPrecompiled is not a public interface.

[zickgraf]

Is there a reason why you have kept the constructor CategoryOfFinSets instead of renaming it to CategoryOfFiniteSets?

[mohamed-barakat]

Is there a reason why you have kept the constructor CategoryOfFinSets instead of renaming it to CategoryOfFiniteSets?

This was an oversight. Done.

[mohamed-barakat]

I agree, CategoryOfSkeletalFinSetsWithMorphismsGivenByListsPrecompiled is not a public interface.

Good.