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

Make imports shortcuts #554

Closed
wants to merge 22 commits into from
Closed

Conversation

mimi030
Copy link

@mimi030 mimi030 commented Apr 30, 2024

**Amendments Overview:**

This pull request introduces the following improvements:

  • Simplified import statements in the 'losses' module.
  • Simplified imports from the 'miners' module.
  • Simplified import paths for the 'models' module.
  • Simplified import statements in the 'samplers' module.
  • Simplified import examples in the 'Features Extraction' documentation.
  • Simplified imports from 'losses', 'models', 'miners', and 'samplers' within the '/oml' module.
  • Added dependencies installation info to contribution guide

**Notes:**

By moving import statements into the corresponding modules' init.py files, most imports are simplified. A few import statements remain unchanged to prevent circular import issues.

Additionally, the import examples in the 'Features Extraction' documentation in Colab require permission to proceed, so they remain unchanged.

@AlekseySh
Copy link
Contributor

Hey, @mimi030
After you solve the conflicts, we can run CI/CD

@AlekseySh
Copy link
Contributor

Also, please, double check contribution pipeline & see how to update Readme

@AlekseySh
Copy link
Contributor

And just to make sure we are on the same page: it's not necessary to simplify the imports inside the modules. I mean, you can do it, but the final purpose if to simplify examples, so, you can only simplify imports there.

@mimi030
Copy link
Author

mimi030 commented May 10, 2024

And just to make sure we are on the same page: it's not necessary to simplify the imports inside the modules. I mean, you can do it, but the final purpose if to simplify examples, so, you can only simplify imports there.

So just to clarify, should I focus only on updating the import statements in the examples found under Features Extraction? I'm feeling a bit confused about the task scope now, as I had understood from our discussion here that we were looking to implement these changes more broadly.

To ensure I'm on the right track, could you please specify which specific files I should be targeting for these updates?

Thank you for your guidance!

@mimi030
Copy link
Author

mimi030 commented May 10, 2024

Also, please, double check contribution pipeline & see how to update Readme

Certainly! I've previously gone through the Contributing in general and Contributing to documentation, but I'll be happy to double-check everything again to ensure everything is in order. If you have specific areas or updates you'd like me to focus on, please let me know.

@AlekseySh
Copy link
Contributor

from the guide:

## Contributing to documentation
* If you want to change `README.md` you should go to `docs/readme`, change the desired section and then build
  readme via `make build_readme`. *So, don't change the main readme file directly, otherwise tests will fail.*

mimi030 and others added 9 commits May 20, 2024 17:28
Removing duplicates and preserving order in Accumulator
…ds in Accumulator

Reworking datasets & interfaces
* Introduced slightly different interfaces for datasets
* Made a dedicated folder for images datasets. Renamed and moved old datasets. Kept names of the old dataset for backward compatibility.
* Removed `ListDataset`

Accumulator
* Removing duplicates and preserving order in Accumulator
…zed kNN

Introducing container for storing Retrieval Results and memory optimized kNN
Changelog:

- Separated map, precision, cmc and fnmr and pfc metrics (there is no more single function that computes all of them)
- Changed signature of the calc_retrieval_metrics (it works with the top k closest items instead of the full gallery)
- Removed repeated calculations in EmbeddingMetrics when slicing over categories
Minor: improved type checking
@AlekseySh
Copy link
Contributor

Close because of: #535 (comment)

@AlekseySh AlekseySh closed this Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants