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

Improve Is{Constant,Univariate}RationalFunction, add tests #2474

Merged
merged 1 commit into from
May 18, 2018

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented May 17, 2018

  • Change IsConstantRationalFunction and IsUnivariateRationalFunction to
    return false for objects which are not even rational functions
  • Improve UNIVARTEST_RATFUN to deal with some more "trivial" cases before
    giving up
  • Add convenience helpers for multiplying rational numbers by rational
    functions over rings which don't contain the rational numbers,
    complementing the existing helpers doing this for addition
  • Add tests

@codecov
Copy link

codecov bot commented May 17, 2018

Codecov Report

Merging #2474 into master will increase coverage by 0.07%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master    #2474      +/-   ##
==========================================
+ Coverage   74.06%   74.14%   +0.07%     
==========================================
  Files         484      484              
  Lines      245498   245545      +47     
==========================================
+ Hits       181830   182057     +227     
+ Misses      63668    63488     -180
Impacted Files Coverage Δ
lib/ratfun1.gi 84.2% <100%> (+5.94%) ⬆️
lib/ratfun.gi 87.36% <80%> (+11.9%) ⬆️
hpcgap/lib/ratfun.gi 76.33% <80%> (+15.2%) ⬆️
hpcgap/lib/polyrat.gi 57.52% <0%> (-9.74%) ⬇️
lib/polyrat.gi 75.5% <0%> (ø) ⬆️
src/io.c 73.53% <0%> (+0.04%) ⬆️
src/stringobj.c 83.45% <0%> (+0.1%) ⬆️
src/objset.c 84.82% <0%> (+0.22%) ⬆️
lib/ratfunul.gi 78.02% <0%> (+3.61%) ⬆️
... and 2 more

Copy link
Contributor

@ssiccha ssiccha left a comment

Choose a reason for hiding this comment

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

LGTM

@fingolfin
Copy link
Member Author

I just extended this some more, adding helpers for multiplying rational numbers with rational functions over e.g. finite fields (we already have such helpers for addition).

@ssiccha
Copy link
Contributor

ssiccha commented May 17, 2018

@fingolfin The helper functions were not added to the HPC-GAP folder. :)

@fingolfin fingolfin force-pushed the mh/test-ratfun branch 3 times, most recently from 6550857 to b3f78f1 Compare May 17, 2018 21:05
* Change IsConstantRationalFunction and IsUnivariateRationalFunction to
  return false for objects which are not even rational functions
* Improve UNIVARTEST_RATFUN to deal with some more "trivial" cases before
  giving up
* Add convenience helpers for multiplying rational numbers by rational
  functions over rings which don't contain the rational numbers,
  complementing the existing helpers doing this for addition
* Add tests
@fingolfin fingolfin merged commit fe11ed7 into gap-system:master May 18, 2018
@fingolfin fingolfin deleted the mh/test-ratfun branch May 18, 2018 06:55
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: tests issues or PRs related to tests release notes: added PRs introducing changes that have since been mentioned in the release notes labels Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants