Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FiltersObj and FiltersType functions #925

Merged
merged 1 commit into from
Nov 3, 2016

Conversation

markuspf
Copy link
Member

The FiltersType and FiltersObj functions return the list of filters set in a type (of an object).

@codecov-io
Copy link

codecov-io commented Oct 24, 2016

Current coverage is 48.82% (diff: 0.00%)

Merging #925 into master will decrease coverage by 0.05%

@@             master       #925   diff @@
==========================================
  Files           424        424          
  Lines        222143     222145     +2   
  Methods        3428       3428          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits         108589     108470   -119   
- Misses       113554     113675   +121   
  Partials          0          0          

Powered by Codecov. Last update b25d0ed...f958b4b

##
#Y Copyright (C) 1997, Lehrstuhl D für Mathematik, RWTH Aachen, Germany
#Y (C) 1998 School Math and Comp. Sci., University of St Andrews, Scotland
#Y Copyright (C) 2002 The GAP Group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminds me: We should revise our copyright headers. In any case, it doesn't make sense to mark something as Copyright 2002 that is added in 2016.
Of course one can now debate whether this is the copyright of the file, or of all of GAP; but even in that latter case, it should be "2002-2016".

#Y Copyright (C) 2002 The GAP Group
##
## This file implements some additional functions relating to types and
## families.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: this .gd file does not "implement" anything; rather, it contains declarations.

##
#M FiltersObj( <object> )
##
## Return list of filters set in the type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add "... of the object" ?

@@ -13,6 +13,24 @@

#############################################################################
##
#M FiltersType( <type> )
##
## Return list of filters set in the type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is just me, but that sounds weird to me... I'd think it should be either "Return list of filters set in .", if type is meant to refer to the name of the argument, or "Return list of filters sets in the given type." if type is meant to refer to the fact that the argument is supposed to be a type object.

obj -> FiltersType(TypeObj(obj)));

#############################################################################
##
#M NamesFilterShort( flags, max )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, this PR looks fine to me. Better introspection features certainly seems like a useful addition to me, be it to debug the system, or to learn.

* FiltersType returns the list of filters set in the
  passed type
* FiltersObj does the same for the passed object
@markuspf
Copy link
Member Author

Updated to accommodate for @fingolfin's comments.

@fingolfin fingolfin merged commit b3f3601 into gap-system:master Nov 3, 2016
@markuspf markuspf deleted the add-filtersobj branch February 5, 2017 12:31
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants