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

Method for PositionsProperty on non-dense Lists #2021

Merged
merged 1 commit into from
Dec 13, 2017

Conversation

sebasguts
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Dec 13, 2017

Codecov Report

Merging #2021 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2021      +/-   ##
==========================================
+ Coverage      66%      66%   +<.01%     
==========================================
  Files         898      898              
  Lines      273268   273275       +7     
  Branches    12745    12745              
==========================================
+ Hits       180378   180383       +5     
+ Misses      90071    90069       -2     
- Partials     2819     2823       +4
Impacted Files Coverage Δ
lib/list.gi 69.76% <100%> (+0.13%) ⬆️
src/hpc/threadapi.c 34.55% <0%> (-0.19%) ⬇️
src/stats.c 79.43% <0%> (-0.14%) ⬇️
src/funcs.c 76.51% <0%> (ø) ⬆️
src/hpc/thread.c 46.01% <0%> (ø) ⬆️
hpcgap/lib/hpc/stdtasks.g 38.87% <0%> (+0.25%) ⬆️

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Looks good to me (but bonus points if you also add a test case for this)

@sebasguts
Copy link
Member Author

@fingolfin Did so, do I get my bonus points ;)?

@fingolfin
Copy link
Member

@sebasguts sure, and a pony and a rainbow!

@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library labels Dec 13, 2017
@fingolfin fingolfin merged commit 0b8d400 into gap-system:master Dec 13, 2017
@olexandr-konovalov
Copy link
Member

@sebasguts bonus points are awarded only if you will help to fix (50% points if at least identify the reason) of the failure of your newly submitted test when all available packages are loaded: https://travis-ci.org/gap-system/gap-docker-master-testsuite/jobs/316407774

@sebasguts
Copy link
Member Author

Cause QPA has an installation of the function that is installed for arbitrary lists, but does not work on lists with holes. I will provide them a patch.

@fingolfin
Copy link
Member

That patch would consist of them removing their PositionsProperty, right? Because it seems to offer nothing beyond what the built-in PositionsProperty does?

Also, unless they release a fixed version quickly, we could raise the rank of our PositionsProperty artificially.

@sebasguts
Copy link
Member Author

sebasguts commented Dec 15, 2017

Yep, did a PR to remove the functions from QPA.

Update(AK): it is gap-packages/qpa#8

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Jan 22, 2018
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 22, 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: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants