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.

Sign up for GitHub

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

Jump to bottom

main.deepforest.predict_file should be able to take in a dataframe, not just a csv file. #797

Open
bw4sz opened this issue Sep 30, 2024 · 3 comments · May be fixed by #852
Open

main.deepforest.predict_file should be able to take in a dataframe, not just a csv file. #797

bw4sz opened this issue Sep 30, 2024 · 3 comments · May be fixed by #852
Labels
API This tag is used for small improvements to the readability and usability of the python API. good first issue Good for newcomers

Comments

@bw4sz
Copy link
Collaborator

bw4sz commented Sep 30, 2024

Looking at the code base.

https://deepforest.readthedocs.io/en/latest/_modules/deepforest/main.html#deepforest.predict_file

it actually looks like dataset.TreeDataset() is the problem here, because utilities.read_file is definitely flexible enough. The docstring of predict_file should be updated and the argument name should be changed (but a deprecation warning added until 2.0), but its the dataset class, which the user really never sees that often that can be updated. I think this would still be considered a patch.
@bw4sz bw4sz added good first issue Good for newcomers API This tag is used for small improvements to the readability and usability of the python API. labels Sep 30, 2024
@Abhishek-kumar0503
Copy link
Contributor

Abhishek-kumar0503 commented Nov 4, 2024 •
edited
Loading

After read your statement it seems that utilities.read_file is reads the input CSV file and is flexible and the dataset.TreeDataset is takes the data from the CSV file and organizes it for prediction and here the issue: TreeDataset isn't flexible enough to handle different formats or structures of input data.
My solution is to Update TreeDataset to be more flexible, so i make the class of name is TreeDataset:

class TreeDataset:
      def __init__(self, csv_file, root_dir, transforms=None, train=False, column_mapping=None):
          self.root_dir = root_dir
          self.transforms = transforms
          self.train = train
          self.column_mapping = column_mapping or {
              "image_path": "image_path",
              "xmin": "xmin",
              "ymin": "ymin",
              "xmax": "xmax",
              "ymax": "ymax"
          }
       
          # Load the CSV file with flexible handling
          self.annotations = self._load_annotations(csv_file)

      def _load_annotations(self, csv_file):
          # Read the CSV file using the existing utility
          df = utilities.read_file(csv_file)
  
          # Rename columns based on column_mapping, if necessary
          df = df.rename(columns=self.column_mapping)
  
          return df

and update the predict_file
ds = dataset.TreeDataset(csv_file=csv_file, root_dir=root_dir, transforms=None, train=False)

Is this correct?

@henrykironde
Copy link
Contributor

@Abhishek-kumar0503, It looks like you're on the right track. Could you submit a PR?.

@henrykironde henrykironde reopened this Nov 4, 2024
@Abhishek-kumar0503
Copy link
Contributor

Abhishek-kumar0503 commented Nov 4, 2024 •
edited
Loading

ya sure @henrykironde . i set up this project in my laptop. I have one query that if i change or update some function in main.py file where it will display that changes.

@Abhishek-kumar0503 Abhishek-kumar0503 mentioned this issue Nov 16, 2024
Open
@naxatra2 naxatra2 linked a pull request Dec 12, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
API This tag is used for small improvements to the readability and usability of the python API. good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

updated main.deepforest.predict_file for DF usage naxatra2/DeepForest-nax
3 participants
@bw4sz @henrykironde @Abhishek-kumar0503